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

encoding of non float parameter values is not supported #743

Closed
mnumanuyar opened this issue Oct 25, 2022 · 7 comments
Closed

encoding of non float parameter values is not supported #743

mnumanuyar opened this issue Oct 25, 2022 · 7 comments

Comments

@mnumanuyar
Copy link

in mavlink; parameter related messages has value field with type float. However mavlink has support for non float parameters as well. this is satisfied by adding a type field in param messages.

For example to send a param with int type (like mav_sys_id) value field of paramsend message should have int encoding instead of float encoding ('01 00 ' instead of '00 00 80 3f' for value 1)

However I think it is not possible incurrent pymavlink.

Below are some relevant code for paramvaluesend. I encountered this problem with param value send, but it probably exist in other parameter related messages aswell.

relevant code:

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)

in short; value field of param_value_send is encoded by struct.Struct("<f").pack(value) which makes it hard to send int params.

Note: this problem seems to be mavlink related problem instead of library specific problem. (for example node-mavlink has this issue as well)

@junwoo091400
Copy link

This is valuable feedback, what is the cause of this then? Definition of messages in MAVLink?

@mnumanuyar
Copy link
Author

mnumanuyar commented Oct 26, 2022

It is definitly part of it, because every generated library default to float values for parameters. There should be some insentive for additional considiration and some way to allow users to parse the raw payload according to param_type.

By the way i found an ugly workaround :
struct.unpack('f',struct.pack('i',actual_param_value))[0]
using this for the param_value of param_value_send(...) make it so that vaue is serialised as int. not sure if it works for all cases (all possible values, and all other param types) though. maybe this canbe generalised and added as util func?

so with that pymavlink and node-mavlink have not well tested workarounds, and cpp and cs libs has probably well tested workarounds (because of qground px4, ardupilot etc.)

@mnumanuyar
Copy link
Author

mnumanuyar commented Oct 26, 2022

additional info about parameter encoding from mavlink docs: https://mavlink.io/en/services/parameter.html#parameter-encoding

Edit: after rechecking that document i think this problem is already adressed in docs in some way, did not had time to check it in detail. so not sure if it is still relevant

@hamishwillee
Copy link
Contributor

This has been working forever, so yes, I believe any issues have been addressed. The current state is as described in the link above: https://mavlink.io/en/services/parameter.html#parameter-encoding

Essentially there are two ways that flight stacks might encode parameter data. They can either simply cast the parameter value into a float - so the int will become a float. This is what ArduPilot does. There is some potential loss of precision for very large integers.

The other way is to bytewise encode integer data into the float - i.e. there is a union provided (see https://mavlink.io/en/services/parameter.html#bytewise-encoding-mavgen-c-api) which allows you to put the 4 byte integer in memory into the float "as bytes". Of course you need to decode it in the same way because if it is read as a float it will not have the same value as the original integer.
This is what PX4 does.

There are also flags to tell you which system the flight stack is using, but these are only useful in the most recent versions (they were misapplied in PX4 prior to v1.13).

@mnumanuyar
Copy link
Author

thank you :D .
I guess than the float type of parameter value indicates the "default" cast type for the first case (ardupilot)
i hope these two ways cover all use cases (i.e. future languages that would be cool to have a mavlink lib) But in anycase as ou said this has been working forever and changeing anything would break more than it fixes here.

In anycase, this issue seems to be well documented and is working as intended so i am closing it

@padcom
Copy link

padcom commented Oct 28, 2022

Hi guys, I am the maintainer of node-mavlink and I can confirm that the problem is quite extensive. Each parameter can have one of many types (integers of different sizes, floats of different sizes, strings) and those are all dependent on the configuration option that you're trying either to read or write.

My initial idea for node-mavlink was to just leave the parameter decoding to whoever is using it. Then I found out from the original author of pymavlink that it does some kind of detection of what the user of said library wants and allows that. I am no python dev, nor I pretend to be one xD so I won't be able to help you with pymavlink, but I can assure you that node-mavlink's implementation of type-safe and precise handling of configuration values is on the way. It may be some time before the implementation is ready, though.

The idea here is that in the library should provide not a generic option to read/write configuration options but rather a ready-made list of configuration options that when read from the byte stream will automatically be of the appropriate type. The same shall happen when you want to upload a configuration change. There should be no doubt as to what that particular configuration option is, what its type is and how it will be serialized/deserialized.

For that the code generator needs to be updated and for some of the classes implementing mavlink messages there needs to be an option to provide custom implementation of those messages rather than stuffing everything into the generator.

I hope to have something more/less ready by the end of the year, so don't hold your breath. But it's not been forgotten. In fact, it is one of those things that keep me up at night and that limit my abilities to complete a couple of interesting projects that I am working on. One of those is the ability to run a monitoring app strictly in the browser without any other applications. Interesting times ahead!

@hamishwillee
Copy link
Contributor

@padcom I think you're proposing a solution to a problem that does not exist. In support of that, many many implementations of generators for which parameter passing is no problem against hundreds of thousands (?) of deployed drones.

A system can determine what encoding is used at high level from the protocol bit. It can tell what the data type of the parameter is from the field that indicates that in the message. That is all that is needed for reliable encoding/decoding. On top of that the component information protocol layers a standard way to pass other metadata about parameters between flight stacks and a GCS (e.g. description and so on that is useful for display).

There was talk of including the encoding in the messages mavlink/rfcs#18 . This is of interest to ArduPilot because it would ease their migration to using byte-wise encoding.

We'd welcome a proposal, but nothing becomes part of the standard without being broadly reviewed and supported by at least two flight stacks. So a good way to flesh out your ideas is to work with one of the flight stacks and verify that they are willing to take the cost of adopting any required change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants