ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
11.08k stars 17.65k forks source link

DroneCAN virtual serial: reported protocol incorrect #27902

Closed olliw42 closed 2 months ago

olliw42 commented 3 months ago

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

tpwrules commented 3 months ago

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.

olliw42 commented 3 months ago

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 ...

tpwrules commented 3 months ago

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

olliw42 commented 3 months ago

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. :)

tpwrules commented 3 months ago

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.

olliw42 commented 2 months ago

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