ardupilot icon indicating copy to clipboard operation
ardupilot copied to clipboard

DroneCAN virtual serial: reported protocol incorrect

Open olliw42 opened this issue 1 year ago • 6 comments

I've just noted that the protocol field in the uavcan.tunnel.Targetted message which is used for DroneCAN virtual serial support is always set to undefined:

https://github.com/ArduPilot/ardupilot/blob/master/libraries/AP_DroneCAN/AP_DroneCAN_serial.cpp#L89

here a DroneCANTool screen shot for with these settings grafik

grafik

olliw42 avatar Aug 22 '24 17:08 olliw42

The flight controller handles the protocol, the periph just sends and receives bytes. So I don't think the protocol should change, not sure if 255 is the best value to use though.

tpwrules avatar Aug 23 '24 19:08 tpwrules

I'm not saying the protocol should change, I'm saying though that the fc should set the protocol field appropriately. It can't for all since not all are reflected in the dronecan tunnel.Targetted definition, but especially for mavlink 1 and 2 it can set them ...

olliw42 avatar Aug 23 '24 19:08 olliw42

What would the protocol be used for? The client should not interpret it.

tpwrules avatar Aug 23 '24 19:08 tpwrules

The client should not interpret it.

??

but it may legitimately want to interpret it It seems the only scenario you can imagine is a situation where a dronecan-to-serial dongle is connected to a serial-device ... what sense does this in general make? None. It is in general MUCH more elegant to have a dronecan-device thing which does something with the data ...

frankly, that's really a bit of a silly discussion. This message has a protocol field which can be set to mavlink v1, mavlink v2, or gps as of now, and it's four easy lines of code to do that, but there is need for a discussion ...

anyway, reported it. All I wanted to do as I felt it appropriate to do. :)

olliw42 avatar Aug 23 '24 20:08 olliw42

In general, dronecan devices should use dronecan messages rather than tunneled serial data.

If the device itself wanted to parse the data, it would have to document a particular serial_id value and the user would configure that.

tpwrules avatar Aug 23 '24 22:08 tpwrules

don't agree raised a PR, so am closing here

olliw42 avatar Sep 05 '24 08:09 olliw42