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

Add plugin to set params #585

Merged
merged 12 commits into from Nov 15, 2018
Merged

Add plugin to set params #585

merged 12 commits into from Nov 15, 2018

Conversation

julianoes
Copy link
Collaborator

@julianoes julianoes commented Oct 31, 2018

This adds a plugin to set/get "raw" mavlink params.

This has been tested against the E90.

When setting int parameters, we get an error but the param change still works. Hopefully, this will be fixed on the Firmware side: PX4/PX4-Autopilot#10792

JonasVautherin
JonasVautherin previously approved these changes Nov 13, 2018
Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

core/mavlink_parameters.cpp Outdated Show resolved Hide resolved
core/mavlink_parameters.cpp Show resolved Hide resolved
core/system_impl.cpp Outdated Show resolved Hide resolved
This is a refactoring which enables better type checking for parameters.
The main change is that the type now needs to be passed to set_param
which means the user of the API needs to know the type of a param that
should be changed beforehand.

This means that the actual type checking can happen inside the
MAVLinkParameters class for both params that are set and params that are
gotten.
Since the parameter interface has been refactored a bit, we now need to
pass on the parameter type from the xml definition before doing a
get_param.
In the case where we don't know about a parameter from the definition
file, there is no point in trying to get it from the camera anyway. This
is just an error.
This plugin enables setting and getting of raw parameters.
Param names can be 16 chars at most (plus zero termination).
This changes the bool indicating success to an enum for different
results indicating success or different errors.
It's more intuitive to use the actual length (without zero termination)
for the define and then add +1 if required.

It turns out that when doing `strcmp`, we shouldn't compare 17 chars, so
including the zero termination but only 16 chars.
@hamishwillee
Copy link
Collaborator

How did you test PARAM_EXT? I didn't think anything other than QGC and Camera Manager used them.

@hamishwillee
Copy link
Collaborator

And just For my information, is the message flow for PARAM_EXT-* the same as for the normal param protocol (though obviously with the new messages)?

@julianoes
Copy link
Collaborator Author

How did you test PARAM_EXT?

Yuneec E90 😄

And just For my information, is the message flow for PARAM_EXT-* the same as for the normal param protocol (though obviously with the new messages)?

It's pretty much the same except that you also have the ack/nack that you can return. @dogmaphobic would know the details.

@julianoes julianoes merged commit 3c7ac25 into develop Nov 15, 2018
@julianoes julianoes deleted the add-raw-params branch November 15, 2018 07:29
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

Successfully merging this pull request may close these issues.

None yet

3 participants