d-ronin / dRonin

The dRonin flight controller software.
http://dronin.org
Other
289 stars 167 forks source link

MSPUAVOBridge: MSP error message differs from *flight #2247

Closed tracernz closed 5 years ago

tracernz commented 5 years ago

This was raised in a BLHeli issue: https://github.com/bitdump/BLHeli/issues/324#issuecomment-477249163. The MSP code sends a message with | in the case of an unknown message type[1]. That is consistent with the Multiwii protocol spec[2], but differs from the actual multiwii/baseflight/betaflight implementations, which send ![3][4][5]. Since we're on our own here, I think we need to disregard the original protocol spec.

[1] https://github.com/d-ronin/dRonin/blob/7b44093d461d073eba52d8d9558a4e56769fd4b3/flight/Modules/UAVOMSPBridge/UAVOMSPBridge.c#L238 [2] http://www.multiwii.com/forum/viewtopic.php?f=8&t=1516 [3] https://github.com/multiwii/multiwii-firmware/blob/upstream_shared/Protocol.cpp#L127 [4] https://github.com/multiwii/baseflight/blob/master/src/serial.c#L214 [5] https://github.com/betaflight/betaflight/blob/8e10e735807945dcf53a3a563085a2df91100355/src/main/msp/msp_serial.c#L305

mikeller commented 5 years ago

I would agree with your approach to disregard the original protocol specification (I had searched for the original MSP specification before, but not found it, so this is the first time I've seen this). Most of the currently used MSP implementations (in particular when counted by installed base) seem to be using '!' to indicate error / unknown command. I think trying to change this for every client / server implementation would just cause disruption for not much benefit, so switching to use '!' for current implementations is probably better. See also https://github.com/iNavFlight/inav/wiki/MSP-V2 for a specification of the protocol currently implemented by Betaflight / cleanflight / iNav.

tracernz commented 5 years ago

BLHeliSuite now supports our old behaviour: https://github.com/bitdump/BLHeli/issues/324#issuecomment-481812307