UAVCAN Rangefinder Support

[avionicsanonymous]

Describe problem solved by this pull request
Add support for rangefinder connected via UAVCAN.

Describe your solution
Added sensor bridge for "rangefinder" sensor, which accepts the included AvAnon rangefinder message. This custom message is added because the standard UAVCAN rangefinder message does not include the necessary data to fill out the PX4 distance_sensor topic, such as the min and max range of the connected sensor.

Describe possible alternatives
The standard UAVCAN rangefinder message could be used, but we would have to come up with some other way to convey the limits of the rangefinder. Since I'm not aware of any currently-existing devices that use that message, this should be just as good. If any exist in the future, we can add handlers for that message to this sensor bridge and solve the missing data at that time.

Test data / coverage
Bench and flight tested using the AvAnon rangefinder UAVCAN interface. "distance_sensor" data was logged as expected.

[avionicsanonymous]

I see the Jenkins SIL tests failed, but can't quite glean why, and can't fathom how CAN driver additions could cause only a tailsitter SIL test to fail anyway?

[hamishwillee]

Cool! Is there any particular rangefinder this has been tested with, and if so, can we have a page for that in the user guide? http://docs.px4.io/master/en/sensor/rangefinders.html

[dagar]

I'm not sure about carrying custom DSDL for this. What about using uavcan::equipment::range_sensor::Measurement and instead we add parameters to set the min and max?

[avionicsanonymous]

I'm not sure about carrying custom DSDL for this. What about using uavcan::equipment::range_sensor::Measurement and instead we add parameters to set the min and max?

I considered something like that, but it seemed more counter to the existing scheme where the user shouldn't have to sort that out themselves. Also, what if there are multiple rangefinders with different ranges? Then you'd not only need multiple sets of params, but those values become node-id dependent. One of the advantages of the CAN-connected rangefinder is that it enables multiple/redundant sensors, so limiting it to one would be a shame.

Thoughts?

[avionicsanonymous]

Cool! Is there any particular rangefinder this has been tested with, and if so, can we have a page for that in the user guide? http://docs.px4.io/master/en/sensor/rangefinders.html

It was developed and tested around this unit, which acts as an interface between the autopilot and many common rangefinders: https://www.tindie.com/products/avionicsanonymous/uavcan-laser-altimeter-interface/

I'd love to add that to the guide!

[dagar]

I considered something like that, but it seemed more counter to the existing scheme where the user shouldn't have to sort that out themselves. Also, what if there are multiple rangefinders with different ranges? Then you'd not only need multiple sets of params, but those values become node-id dependent. One of the advantages of the CAN-connected rangefinder is that it enables multiple/redundant sensors, so limiting it to one would be a shame.

Thoughts?

I agree with everything you said, I'm just trying to weigh it against the potential harm of custom DSDL for something relatively common. The quick and dirty solution is to carry that metadata in the PX4 uavcan module, then if the need arises for more than one rangefinder over UAVCAN with different metadata we'd have to invent something.

Then we make sure we get it right for UAVCANv1 (a large ongoing effort).

[avionicsanonymous]

I considered something like that, but it seemed more counter to the existing scheme where the user shouldn't have to sort that out themselves. Also, what if there are multiple rangefinders with different ranges? Then you'd not only need multiple sets of params, but those values become node-id dependent. One of the advantages of the CAN-connected rangefinder is that it enables multiple/redundant sensors, so limiting it to one would be a shame.
Thoughts?

I agree with everything you said, I'm just trying to weigh it against the potential harm of custom DSDL for something relatively common. The quick and dirty solution is to carry that metadata in the PX4 uavcan module, then if the need arises for more than one rangefinder over UAVCAN with different metadata we'd have to invent something.

Then we make sure we get it right for UAVCANv1 (a large ongoing effort).

Fair enough - and I suppose the most likely scenario if you have redundant rangefinders is that they're identical anyway. I'll make those changes.

[avionicsanonymous]

@dagar , where do you suggest putting the param definitions? Within the UAVCAN driver param set (which are currently mostly CAN server-centric), with the sensor params, or...?

[hamishwillee]

When this is in it will need documentation. At high level that means "enough for someone else to use it".
At lower level that usually means a page below here that explains how to set up the rangefinder of the particular type.

You might also add info in the devguide if there are particular things that someone wanting to add uavcan support to their own rangefinder needs to know.

[avionicsanonymous]

@hamishwillee , Thanks for the suggestion. I have the documentation ready to go here: PX4/PX4-user_guide#756

@dagar , I think this is good to go. Let me know if you have any other concerns!

[avionicsanonymous]

Rebased on current master and refactored to use PX4Rangefinder.

[JacobCrabill]

Looks good; tested it and it works. Can be merged as-is, though adding the sensor_type would be the final touch.

[avionicsanonymous]

@dagar do you want to see anything else or is this ready to merge?

[avionicsanonymous]

@dagar sorry to poke you again, but when can we merge this? I have a few of these devices in the wild with users who don't build from source but want to be able to use them!

[hamishwillee]

@avionicsanonymous Do you send max/min range in UAVCAN somehow? I ask because of this question: #15551

[avionicsanonymous]

@avionicsanonymous Do you send max/min range in UAVCAN somehow? I ask because of this question: #15551

Yes - this PR includes two new parameters, UAVCAN_RNG_MIN and UAVCAN_RANGE_MAX, which are to be set to match the rangefinder's min and max range. I believe I addressed this in the WIP documentation PR as well, but will go make sure of that. I originally added a custom message definition that allowed the node to send its own min/max range but was asked to remove that in favor of using the stock message and add parameters instead.

Think we can get this merged soon? Evidently I'm not the only one who needs it!

[hamishwillee]

@avionicsanonymous Thanks

Think we can get this merged soon? Evidently I'm not the only one who needs it!

That's up to @dagar or some other member of the core dev team. Dan, is there someone else who might review the changes to get this in?

[avionicsanonymous]

Updated to fix merge conflicts with master, which keeps running away from this PR. A couple sim tests are failing but I doubt that has anything to do with the relevant changes here.

@dagar , can we please merge this soon so I don't have to keep fixing it to stay up to date?

[avionicsanonymous]

@dagar , what can I do to help get this merged? Thanks!

[dagar]

@dagar , what can I do to help get this merged? Thanks!

Sorry for the delay, I missed that the issue with using custom DSDL had been resolved. Let's get this in and make sure that the proper metadata and instance handling is in place as we get closer to UAVCANv1.