search

christianrauch / msp

Implementation of the MultiWii Serial Protocol (MSP) for MultiWii and Cleanflight flight controller
http://www.multiwii.com/wiki/index.php?title=Multiwii_Serial_Protocol
GNU Lesser General Public License v3.0
73 stars 26 forks source link

wrong scaling of raw values in messages #47

Closed christianrauch closed 3 years ago

christianrauch commented 3 years ago

The MSP version 2 support in #28 introduced different methods for de-/serialisation with integrated scaling.

Some of the scalings got lost (e.g. in MSP_ATTITUDE) and some had their meaning inverted (e.g. MSP_ANALOG). The raw battery value (vbat in MSP_ANALOG) specifies a multiplier of 1/10 (unit: 1/10 volt).

In the code, this is using a scale of 0.1: https://github.com/christianrauch/msp/blob/c8f638a078e2d16af74b6c9b865e9d9b9e859a48/inc/msp/msp_msg.hpp#L3133 but this scale is applied as divisor: https://github.com/christianrauch/msp/blob/c8f638a078e2d16af74b6c9b865e9d9b9e859a48/inc/msp/ByteVector.hpp#L328 and not as a multiplier. This leads to a 100 fold value of the correct battery voltage (e.g. 410V instead of 4.1V).

@humanoid2050 Could you clarify how you intended to apply the scale? Is it supposed to be a multiplier, in which case we have to change the unpack method, or is it supposed to be a divisor, in which case we have to update the messages?

humanoid2050 commented 3 years ago

Been a while since I looked at this code. I think I may have messed up the scale factor in the message itself. The goal was to have the pack/unpack be symmetric. So if the value should be scaled by 10 when packing (multiplied), it should be unscaled by 10 (divided) when unpacking.

christianrauch commented 3 years ago

I haven't used the driver for a while and just recognised the weird numbers now. From the documentation, it seems that the division by scale for unpack and multiplication by scale for pack is correct but that some of this got lost when porting the messages. The safe solution would be to just apply any additional transformations inside the message definition, and not leave it to the de-/serialiser.

I fixed some of the message definitions in #48 but might have missed some definitions that I am not using.

Did you recognise any weird values in your application?

humanoid2050 commented 3 years ago

I don't have the hardware hooked up right now to test. Sorry. Might be a while before I can get back to it.