Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ParamValueTypes #11

Open
mnumanuyar opened this issue May 20, 2022 · 9 comments
Open

ParamValueTypes #11

mnumanuyar opened this issue May 20, 2022 · 9 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mnumanuyar
Copy link

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
Copy link
Collaborator

padcom commented May 20, 2022

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.

@padcom padcom added enhancement New feature or request help wanted Extra attention is needed labels May 20, 2022
@mnumanuyar
Copy link
Author

btw This is very important for setparamvalue messages of int parameters

@padcom
Copy link
Collaborator

padcom commented May 20, 2022 via email

@mnumanuyar
Copy link
Author

mnumanuyar commented May 20, 2022

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
Copy link
Collaborator

padcom commented May 20, 2022

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
Copy link
Author

mnumanuyar commented May 21, 2022

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
Copy link
Author

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
Copy link
Author

i got an answer, as it turns out this is the intended way.
ArduPilot/pymavlink#743 (comment)

Unless you want to add something this issue can be closed

@padcom
Copy link
Collaborator

padcom commented Jul 23, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants