feat(inputs): Add Mavlink input plugin #16221

chrisdalke · 2024-11-22T12:48:30Z

Summary

Add a Mavlink input plugin.

The mavlink plugin connects to a MavLink-compatible flight controller such as as ArduPilot or PX4. and translates all incoming messages into metrics.

The purpose of this plugin is to allow Telegraf to be used to ingest live flight metrics from unmanned systems (drones, planes, boats, etc.)

Telegraf is already often used on flight computers (eg a Raspberry Pi) to collect system metrics for drones and it would be valuable to extend this to also provide a convenient way to record flight telemetry.

TODO

gomavlib is currently on a fork to support reading serial ports on i386 and darwin. Will need to upstream the fix for this and move back to the main repo after merge, or before merge if the forked gomavlib is not acceptable

Checklist

No AI generated code was used in this PR

Related issues

Resolves feat(inputs): Add Mavlink input plugin #16220

…to feature/mavlink-input-plugin

…nput-plugin

Hipska · 2024-11-27T08:05:04Z

plugins/inputs/mavlink/mavlink.go

+		endpointConfig, err := ParseMavlinkEndpointConfig(s)
+		if err != nil {
+			s.Log.Debugf("%s", err.Error())
+			return
+		}


Config validation should be done in func (s *Mavlink) Init() error method.

Fixed, move this code into Init.

Hipska · 2024-11-27T08:06:01Z

plugins/inputs/mavlink/mavlink.go

+}
+
+func (s *Mavlink) Start(acc telegraf.Accumulator) error {
+	s.acc = acc


Not really necessary to copy the var I think,

Hipska · 2024-11-27T08:11:58Z

plugins/inputs/mavlink/sample.conf

+     # REQUEST_DATA_STREAM or MAV_CMD_SET_MESSAGE_INTERVAL
+     # (See https://mavlink.io/en/mavgen_python/howto_requestmessages.html#how-to-request--stream-messages)
+     stream_request_enable = true
+     stream_request_frequency = 4


There is no EOL at EOF

Hipska · 2024-11-27T08:12:32Z

plugins/inputs/mavlink/sample.conf

@@ -0,0 +1,37 @@
+# Read metrics from a Mavlink connection to a flight controller.
+[[inputs.mavlink]]
+     # Flight controller URL. Must be a valid Mavlink connection string in one


Comments need to have double escaped: ##

Hipska · 2024-11-27T08:14:44Z

plugins/inputs/mavlink/README.md

+### Note: Mavlink Dialects
+
+This plugin currently only uses the ArduPilot-specific dialect, which also
+includes messages from the common Mavlink dialect.
+
+See the [Mavlink docs](https://mavlink.io/en/messages/ardupilotmega.html) for
+more info on dialects.


Is this intended to be a sub-section of Configuration?

I've just removed this and merged it into the copy in the plugin summary at the top, doesn't need its own section.

Hipska · 2024-11-27T08:22:44Z

plugins/inputs/mavlink/mavlink.go

+// Container for a parsed Mavlink frame
+type metricFrameData struct {
+	name   string
+	tags   map[string]string
+	fields map[string]any
+}


At this point, you can just use telegraf.Metric as that will be just the same?

Removed this type entirely in favor of passing around telegraf.Metric

Hipska · 2024-11-27T08:28:02Z

plugins/inputs/mavlink/mavlink_utils.go

+	for i := 0; i < t.NumField(); i++ {
+		field := t.Field(i)
+		value := v.Field(i)
+		out.fields[ConvertToSnakeCase(field.Name)] = value.Interface()


I don't think it is needed to do this conversion, Telegraf is perfectly fine with CamelCase field and metric names.

I'd like to keep this around, there's a lot of different conventions in mavlink message naming and not much consistency. The Mavlink docs use capitalized snake case like MESSAGE_GLOBAL_POSITION_INT, gomavlib converts that to MessageGlobalPositionInt, and some other client libraries use normal snake case like message_global_position_int.

My argument for using lowercase snake case is that, with a lack of one definite standard, I suspect by convention most people will want the outputs to be snake case in their database (Influx, Postgres, etc).

Okay, fine for me 👍

Hipska · 2024-11-27T08:29:47Z

plugins/inputs/mavlink/mavlink_utils.go

+// Check if a string is in a slice
+func Contains(slice []string, str string) bool {
+	for _, item := range slice {
+		if item == str {
+			return true
+		}
+	}
+	return false
+}


Use internal choice.Contains instead.

Thanks, updated

Hipska · 2024-11-27T08:32:36Z

plugins/inputs/mavlink/mavlink.go

+			if err != nil {
+				s.Log.Debugf("Mavlink failed to connect (%s), will try again in 5s...", err.Error())
+				time.Sleep(5 * time.Second)
+				continue
+			}


Simply return a internal.StartupError and let Telegraf handle the reconnect.

Updated so the Mavlink connection happens outside the loop and throws StartupError. One caveat I do see about doing it this way is that this causes Telegraf to shutdown if Mavlink client initialization fails, so we could lose the metrics from other plugins if we're rebooting waiting for the USB device which may not turn on at the same time as the companion computer.

That said, this condition actually only happens in serial port mode, where the plugin is looking for a serial device. In TCP or UDP mode the client internally handles reconnection so this won't cause the client to fail.

Hipska · 2024-11-27T08:36:54Z

plugins/inputs/mavlink/mavlink_utils.go

+	if strings.HasPrefix(s.FcuURL, "serial://") {
+		tmpStr := strings.TrimPrefix(s.FcuURL, "serial://")
+		tmpStrParts := strings.Split(tmpStr, ":")
+		deviceName := tmpStrParts[0]


Go builtin net/url has useful Parse method to facilitate this easier..

Updated to remove strings.HasPrefix(s.FcuURL, "serial://") and use url.Parse instead. There's still a little string find-and-replace here, because the serial port paths have forward slashes in them, which seems to mess with the URL parsing.

telegraf-tiger · 2024-11-29T18:24:56Z

Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz

Hipska

Going in good direction!

Hipska · 2024-12-02T09:27:46Z

plugins/inputs/mavlink/mavlink.go

+	// Parse out the Mavlink endpoint.
+	endpointConfig, err := ParseMavlinkEndpointConfig(s)
+	if err != nil {
+		return fmt.Errorf("%s", err.Error())


Simply return the error? Why wrapping the message in a new string to be added as error?

Hipska · 2024-12-02T09:28:59Z

plugins/inputs/mavlink/mavlink_utils.go

+func ParseMavlinkEndpointConfig(s *Mavlink) ([]gomavlib.EndpointConf, error) {
+	u, err := url.Parse(s.FcuURL)
+	if err != nil {
+		return nil, errors.New("invalid fcu_url")


Suggested change

return nil, errors.New("invalid fcu_url")

return nil, fmt.Errorf("invalid fcu_url: %w", err)

Hipska · 2024-12-02T09:29:27Z

plugins/inputs/mavlink/mavlink_utils.go

+		if len(tmpStrParts) == 2 {
+			newBaudRate, err := strconv.Atoi(tmpStrParts[1])
+			if err != nil {
+				return nil, errors.New("serial baud rate not valid")


Suggested change

return nil, errors.New("serial baud rate not valid")

return nil, fmt.Errorf("serial baud rate not valid: %w", err)

And similar for all following error returns..

Hipska · 2024-12-02T09:34:46Z

plugins/inputs/mavlink/mavlink_utils.go

+		tmpStr := strings.TrimPrefix(s.FcuURL, "tcp://")
+		tmpStrParts := strings.Split(tmpStr, ":")
+		if len(tmpStrParts) != 2 {


Please use net/url utilities

Suggested change

tmpStr := strings.TrimPrefix(s.FcuURL, "tcp://")

tmpStrParts := strings.Split(tmpStr, ":")

if len(tmpStrParts) != 2 {

if u.Port() == "" {

Hipska · 2024-12-02T09:37:49Z

plugins/inputs/mavlink/mavlink_utils.go

+		hostname := tmpStrParts[0]
+		port, err := strconv.Atoi(tmpStrParts[1])
+		if err != nil {
+			return nil, errors.New("tcp port is invalid")
+		}


No need to convert port to int to convert it to string again later..

Suggested change

hostname := tmpStrParts[0]

port, err := strconv.Atoi(tmpStrParts[1])

if err != nil {

return nil, errors.New("tcp port is invalid")

}

hostname := u.Hostname()

port := u.Port()

(validity of port number already done in url.Parse.)

Hipska · 2024-12-02T09:45:30Z

plugins/inputs/mavlink/mavlink.go

+				if len(s.MessageFilter) > 0 && !choice.Contains(result.Name(), s.MessageFilter) {
+					continue
+				}
+				result.AddTag("fcu_url", s.FcuURL)


We normally use source as tag name for indicating from where a metric arrived.

Hipska · 2024-12-02T09:46:09Z

plugins/inputs/mavlink/mavlink.go

+	})
+	if err != nil {
+		return &internal.StartupError{
+			Err:   fmt.Errorf("mavlink client failed (%w)", err),


Suggested change

Err: fmt.Errorf("mavlink client failed (%w)", err),

Err: fmt.Errorf("mavlink client failed: %w", err),

Hipska · 2024-12-02T09:47:57Z

plugins/inputs/mavlink/sample.conf

+
+     ## Filter to specific messages. Only the messages in this list will be parsed.
+     ## If blank or unset, all messages will be accepted.
+     # message_filter = ["global_position_int", "attitude"]


Commented out paramaters should contain the default value. This seems not to be the case here?

Hipska · 2024-12-02T09:57:49Z

plugins/inputs/mavlink/mavlink_utils.go

+		if len(hostname) > 0 {
+			return []gomavlib.EndpointConf{
+				gomavlib.EndpointTCPClient{
+					Address: fmt.Sprintf("%s:%d", hostname, port),


This could simply be

Suggested change

Address: fmt.Sprintf("%s:%d", hostname, port),

Address: u.Hostname,

As you checked before it has port number..

Hipska · 2024-12-02T10:00:11Z

plugins/inputs/mavlink/mavlink_utils.go

+}
+
+// Parse the FcuURL config to setup a mavlib endpoint config
+func ParseMavlinkEndpointConfig(s *Mavlink) ([]gomavlib.EndpointConf, error) {


If this is the only value that is used in this function, then maybe just only pass that.

Suggested change

func ParseMavlinkEndpointConfig(s *Mavlink) ([]gomavlib.EndpointConf, error) {

func ParseMavlinkEndpointConfig(fcuURL string) ([]gomavlib.EndpointConf, error) {

chrisdalke and others added 6 commits November 21, 2024 08:31

Mavlink test

290d0bf

Working threaded mavlink plugin

8d95aac

fix config

8105d40

Support all client types

24d47db

wip

89264c4

fix

084395f

telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Nov 22, 2024

chrisdalke and others added 13 commits November 22, 2024 07:54

Fix README

12c598f

Fixing README

b8b5030

Fixes

cb3b1b6

Lint

93899a1

Remove unused imports for now

bc756ea

Fix

1151fda

Add some tests, split things into util file

a85be7a

Working tests

4d8de2c

Lint fixes

18e2180

More fixes

24f8c06

Fix problems

9c07968

Fix dependencies

c9f3c80

Test with cross platform fork of gomavlib

b368ccb

chrisdalke mentioned this pull request Nov 25, 2024

Consider changing serial library to improve multiplatform support bluenviron/gomavlib#130

Open

chrisdalke and others added 7 commits November 24, 2024 19:36

Fix dependency license file

c656844

Don't export the metric frame data type

ba1f7e5

Merge remote-tracking branch 'origin/feature/mavlink-input-plugin' in…

231b70a

…to feature/mavlink-input-plugin

wip

7579342

Revert dep changes that had nothing to do with this PR

0c6a561

Fix

66adb19

Merge remote-tracking branch 'upstream/master' into feature/mavlink-i…

fbf605c

…nput-plugin

Hipska suggested changes Nov 27, 2024

View reviewed changes

WIP PR review

56e345d

cdalke-havoc added 4 commits November 29, 2024 12:24

Fix README example output, message filtering

2d170ef

Lint fixes

3b504e5

Move client configuration to Init, connection to Start

e8db31b

Lint + test fixes

2c1dfd6

Hipska suggested changes Dec 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(inputs): Add Mavlink input plugin #16221

feat(inputs): Add Mavlink input plugin #16221

chrisdalke commented Nov 22, 2024 •

edited

Loading

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024

Hipska Nov 27, 2024

chrisdalke Nov 29, 2024 •

edited

Loading

telegraf-tiger bot commented Nov 29, 2024

Artifact URLs

Hipska left a comment

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

Hipska Dec 2, 2024

	return nil, errors.New("invalid fcu_url")
	return nil, fmt.Errorf("invalid fcu_url: %w", err)

	return nil, errors.New("serial baud rate not valid")
	return nil, fmt.Errorf("serial baud rate not valid: %w", err)

	Err: fmt.Errorf("mavlink client failed (%w)", err),
	Err: fmt.Errorf("mavlink client failed: %w", err),

	Address: fmt.Sprintf("%s:%d", hostname, port),
	Address: u.Hostname,

	func ParseMavlinkEndpointConfig(s *Mavlink) ([]gomavlib.EndpointConf, error) {
	func ParseMavlinkEndpointConfig(fcuURL string) ([]gomavlib.EndpointConf, error) {

feat(inputs): Add Mavlink input plugin #16221

Are you sure you want to change the base?

feat(inputs): Add Mavlink input plugin #16221

Conversation

chrisdalke commented Nov 22, 2024 • edited Loading

Summary

TODO

Checklist

Related issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisdalke Nov 29, 2024 • edited Loading

Choose a reason for hiding this comment

telegraf-tiger bot commented Nov 29, 2024

Artifact URLs

Hipska left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisdalke commented Nov 22, 2024 •

edited

Loading

chrisdalke Nov 29, 2024 •

edited

Loading