ArduPilot / node-mavlink

This project is providing native TypeScript bindings and tools for sending and receiving MavLink messages over a verity of medium
GNU Lesser General Public License v3.0
75 stars 25 forks source link

ParamValueTypes #11

Open mnumanuyar opened 2 years ago

mnumanuyar commented 2 years ago

What is the correct way to parse the value of ParamValue? In the docs it is stated that actual type of param_value fields are indicated in param_type fields. Right now we can parse the correct values via something like :

            const data = packet.protocol.data(packet.payload, common.ParamValue);

            let typedVal = NaN
            switch (data.paramType) {
                case common.MavParamType.INT32:
                    typedVal = packet.payload.readInt32LE()
                    break;
                case common.MavParamType.REAL32:
                    typedVal = packet.payload.readFloatLE()
                    break;

                default:
                    break;

1- Is this the correct way? 2- can it be done inside node-mavlink? 3- I dont know if there are other messages like that, but if so passing on the raw value of fields might be helpfull. In this case since paramValue is at the start of payload; code is clean but if it were to at the middle of the payload, we might have had to search for the correct offset etc.

padcom commented 2 years ago

Currently, there is no other way to read param_value with the correct type. I am planning on extending the generator to have the ParamValue class pre-generated and being able to read/write the correct type of value. What remains uncertain is how to approach writing ParamValue.

If you have an idea how we could approach it please let me know.

mnumanuyar commented 2 years ago

btw This is very important for setparamvalue messages of int parameters

padcom commented 2 years ago

I know, this is a shortcoming at the moment.

pt., 20 maj 2022 o 13:46 numan @.***> napisał(a):

btw This is very important for setparamvalue messages of int parameters

— Reply to this email directly, view it on GitHub https://github.com/ArduPilot/node-mavlink/issues/11#issuecomment-1132804305, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEHICRN34MQ55WJNB67KMTVK53QFANCNFSM5WOOQU5Q . You are receiving this because you commented.Message ID: @.***>

mnumanuyar commented 2 years ago

I think as a temp solution to set int parameters, this works:


    const message = new common.ParamSet()
    message.targetSystem =1
    message.targetComponent = 1;
    message.paramId = "CAL_AIR_CMODEL";
    message.paramType = 6;
    let tmpBuffer = Buffer.alloc(4)
    tmpBuffer.writeInt32LE(0) 
    message.paramValue = tmpBuffer.readFloatLE();

    await send(port, message, new MavLinkProtocolV2())

of course, the value inside of writeInt32LE is value we want to set, and I have not yet tested this extensively. I will keep you updated if I encounter anything with this "solution" Also I will try to properly solve this, once I have time, and if this is not solved by then :D

padcom commented 2 years ago

I think the biggest problem that I can see here is that the parameter has the value associated with it on the definition level and not in the bytes on the wire (or am I wrong?). With that, it is hard to figure out what type to serialize the data to.

I might think of some generator that will kind of generate a list of properties that are properly typed when a value is assigned to it.

mnumanuyar commented 2 years ago

I think you are absolutely correct. There is no way of parsing paramvalue withouth knowing the definions of paramtype. However at some level that is bound to happen. For example, some of the params are actually enums and their definitions are in parameter metadatafiles of their respected firmwares. I think the question is where to draw the line. So the correct way to handle it is perhaps just passing on the raw values (in addition to existing parsing ofcourse). packet.payload kinda of does that but I think param values never exceeds their defined lengths so passing on the raw values of each field (splited but not parsed version of packet.payload) might be helpfull. (well i guess there is nothing preventing someones dialect to merge two parameters and parse as one) Inspecting other implementations of mavlink might also be usefull.

Or I might just be overthinking it, you are the boss :D

mnumanuyar commented 2 years ago

ok so ... exact same problem exist in pymavlink (well, similiar.but probably exact problem also. and maybe withou workaround) .

relevant code is below. they are taken from ardupilotmega.py after i installed pymavlink, but i donr know where it is in their repo

import struct

def param_value_send(self, param_id, param_value, param_type, param_count, param_index, force_mavlink1=False):
    return self.send(self.param_value_encode(param_id, param_value, param_type, param_count, param_index), force_mavlink1=force_mavlink1)

def param_value_encode(self, param_id, param_value, param_type, param_count, param_index):
      return MAVLink_param_value_message(param_id, param_value, param_type, param_count, param_index)

class MAVLink_param_value_message(MAVLink_message):
      unpacker = struct.Struct("<fHH16sB")
      def pack(self, mav, force_mavlink1=False):
            return self._pack(mav, self.crc_extra, self.unpacker.pack(self.param_value, self.param_count, self.param_index, self.param_id, self.param_type), force_mavlink1=force_mavlink1)

tl;dr: , in pymavlink value of paramvaluesend is serialised by struct.Struct("<f").pack(value) which makes sending correct value very hard for non float values.

I did not check but proably similiar issue exist in node-amvlink param value send, and pymavlink paramvalue messages. (and maybe all param related messages in many auto generated mavlink libs?)

maybe param related messages values should not be float and instead bytearray kind of thing in mavlink :/

mnumanuyar commented 2 years ago

i got an answer, as it turns out this is the intended way. https://github.com/ArduPilot/pymavlink/issues/743#issuecomment-1291549410

Unless you want to add something this issue can be closed

padcom commented 3 months ago

Unless you want to add something this issue can be closed

I am still not done with it and I'll keep this issue open until I get a more humane way of dealing with both reading and setting values for parameters. I'd love to have (at the very least for the known configuration options) a fully type-safe way of dealing with settings. But it requires both time and a good idea how to do it so it makes sense, of which I am short at the moment. One day, hopefully...