UAVCAN improvements

[pavel-kirienko]

Greetings, my fellow humans.

This PR brings the following improvements:

• Configuration parameter UAVCAN_ESC_IDLT, when set to 1, ensures that UAVCAN ESC will be running at a minimum throttle while the system is armed. The default value is 0, so it will not affect existing setups.
• Beeping patterns during ESC enumeration were improved. One video is worth 1080 words: https://youtu.be/4nSa8tvpbgQ
• Fixed HDOP/VDOP not being reported from CAN GPS. #5153 - HDOP and VDOP are populated with the value of PDOP.
• A couple of minor cleanups in the code.

Mentioning @mhkabir and @aperture-laboratories because I want them to stay informed.

[mhkabir]

Standby for flight testing and video for user documentation.

[pavel-kirienko]

Can someone please predict what release these changes will end up in? I'm writing some tutorials and I would like to pinpoint the minimum required version of PX4.

[mhkabir]

Changes were flight tested today on MAV5. Looking good! Everything works like it should, although I had to turn off a couple of drivers to reclaim some RAM. Otherwise the shell wouldn't work.

predict what release these changes will end up in.

@LorenzMeier 1.4.2, I'm sure?
@pavel-kirienko Can you please rebase this on master again?

[mhkabir]

[mhkabir]

Found 2 minor uavcan user experience related issues which should be fixed before release :

1. UAVCAN node parameters do not appear correctly  mavlink/qgroundcontrol#3925
2. With this PR, each uavcan node capable of beeping beeps during the node parameter sync, for example when connecting Qgroundcontrol.

[pavel-kirienko]

Kabir, number 2 is not a bug, it's a feature. I added it in order to provide a simple audio indication that some UAVCAN related activity is happening. If you find it confusing we can easily remove it though (it's one line).

[mhkabir]

Hmm. It's kinda annoying. I don't know why we'd want that when connecting to a GCS. Also, what happens if we're in flight and we connect and the beep commands go through?

I also just found that saving UAVCAN params via QGC isn't working at all. The parameter write fails with a failure message from Firmware. Please check. This is critical for me since the Babel blew, and also in general for a smooth user experience.

[pavel-kirienko]

what happens if we're in flight and we connect and the beep commands go through?

They will be ignored.

I also just found that saving UAVCAN params via QGC isn't working at all.

Thanks, I will try to repro this.

[proficnc]

I like the beeping as is... Found it quite reassuring that all systems were right.

[pavel-kirienko]

I think I identified the problem with parameters. When responding to a parameter list request, the MAVLink-UAVCAN bridge converts the parameter data from uavcan_parameter_value_s to mavlink_param_value_t, losing the source node ID in the process. Looking at the inverse conversion in the parameter set request handler, I observe that the intention was to use the MAVLink Component ID to represent the value of UAVCAN Node ID (which makes sense), where the component ID is defined in the field target_component of the message type PARAM_SET.

The fundamental problem here, as I understand it, is that the message PARAM_VALUE doesn't have a field to indicate what MAVLink Component ID this value was obtained from. I'm not sure how this was even expected to work in the first place. Did we intend to substitute the source component ID when sending PARAM_VALUE with the UAVCAN node ID? If so, the MAVLink library does not seem to support this.

@bendyer

[pavel-kirienko]

@proficnc I would like to keep beeping too, unless there are any UX issues with it I'm not aware about.

[pavel-kirienko]

I propose to have this merged and resolve the parameter bridge issues separately. The branch has been merged with master.

[pavel-kirienko]

I mean, the master has been merged into this branch, sorry about the confusion.

[LorenzMeier]

the MAVLink libs support setting the origin component ID. It might need some tweaks on access but I can enable that certainly.

And yes, that's how it's supposed to work. We also should send a heartbeat for each subsystem with a different component ID.

[LorenzMeier]

The CI systems still need to pass

[bendyer]

@pavel-kirienko This was originally able to work because the UAVCAN node ID was passed in as the component ID parameter in _mavlink->send_msg: https://github.com/PX4/Firmware/pull/2981/files#diff-1a12062ccb751d0439a211a0cddce073R280

I believe your assessment of why it's not currently working is correct.

Looks like it might be possible to fix the issue without changing the mavlink_param_value_t definition by switching to some combination of mavlink_msg_param_value_encode_chan, mavlink_finalize_message_chan and _mavlink_resend_uart.

[pavel-kirienko]

Build fixed by updating to a newer commit in master (it was broken there). I think I noticed that hysteresis and autodeclination tests are still sometimes failing randomly though.

[pavel-kirienko]

CI reports are misleading and should be ignored - the master is still broken at the moment. This PR is ready for merging.

[pavel-kirienko]

Nudge.

[mhkabir]

@LorenzMeier This is good to go. Flight tested a couple of times.

@pavel-kirienko How are we looking on the parameter bridge fix?

[pavel-kirienko]

We need Lorenz to enable the origin component id setting in the MAVLink library. Rest is a piece of cake.

[NosDE]

@LorenzMeier @pavel-kirienko flight looks good !
https://youtu.be/ZNsAkfqkHYQ

[mhkabir]

@LorenzMeier Can we get this in now?

It would also be great if you could quickly enable the required feature in mavlink to allow the parameter bridge to function correctly (#5277). I'm having some issues with aggressive control on the Orels, and need to tune the control parameters on the ESCs, which is impossible without the proper (or working, for that reason) parameter interface.

[LorenzMeier]

Thanks everybody! Rebased and applied! Now on to fixing the MAVLink param issue..