Refactoring of the MAVLink parameter interface #449
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianoes
force-pushed
the
refactor-params
branch
from
7807616
to
f9da1ba
Compare
julianoes
force-pushed
the
refactor-params
branch
from
f9da1ba
to
2ae2c9c
Compare
julianoes
force-pushed
the
refactor-params
branch
2 times, most recently
from
bf6ea37
to
bbd760a
Compare
julianoes
changed the title
|
Ready for review. |
julianoes
force-pushed
the
refactor-params
branch
from
bbd760a
to
96ca9e2
Compare
|
@JonasVautherin I don't think it should break it? Which part would be broken? |
julianoes
force-pushed
the
refactor-params
branch
from
96ca9e2
to
102add7
Compare
|
@JonasVautherin would be good to get this one in. |
JonasVautherin
requested changes
Jul 24, 2018
julianoes
force-pushed
the
refactor-params
branch
from
102add7
to
cb152bb
Compare
julianoes
commented
Jul 26, 2018
| static_cast<rpc::action::ActionResult::Result>(action_result)); | ||
| // rpc_action_result->set_result_str(action_result_str(action_result)); | ||
| response->set_allocated_action_result(rpc_action_result); | ||
| if (response != nullptr) { |
|
|
||
| actionService.SetMaximumSpeed(nullptr, nullptr, nullptr); | ||
| actionService.SetMaximumSpeed(nullptr, nullptr, &response); |
| actionService.SetMaximumSpeed(nullptr, nullptr, &response); | ||
| } | ||
|
|
||
| TEST_F(ActionServiceImplTest, setMaxSpeedDoesNotCrashWithNullResponse) |
JonasVautherin
force-pushed
the
refactor-params
branch
from
601a2c2
to
3456e9d
Compare
We don't need to store params anymore, they are always auto-saved.
This adds a cache for the params so that when you want to get a parameter, it takes it from the cache if you have received it before. This will only really be beneficial once plugins can require params right when they are instantiated respectively when a vehicle is connected.
We should not assume that the firmware is already set to the default takeoff altitude because it might have been changed in the past.
We can't always be sure if settings via commands work, so we do need to supply a way to check for errors.
Also get rid of some unnecessary comments and move the param get methods so they are together.
Instead of setting a default takeoff altitude just rely on the fact that the drone itself has a takeoff altitude set by a parameter. This should avoid surprises where taking off with the SDK has a different altitude than with other ground stations.
These are not needed anymore because the param system should cache it for us.
This way we can get rid of some future/promise duplication.
Since we now have an API to set params sync instead of async we can simplify the way the configuration is applied. Additionally, it does not really make sense to change the PX4 default without asking every time the plugin connects. We should rather fetch the params in the beginning and have them cached for when we need to change them.
JonasVautherin
force-pushed
the
refactor-params
branch
from
3456e9d
to
2d68e6d
Compare
julianoes
force-pushed
the
refactor-params
branch
from
2d68e6d
to
7f84322
Compare
JonasVautherin
force-pushed
the
refactor-params
branch
from
7f84322
to
29aa760
Compare
JonasVautherin
approved these changes
Jul 26, 2018
rt-2pm2
pushed a commit
to rt-2pm2/DronecodeSDK
that referenced
this pull request
Nov 27, 2018
Refactoring of the MAVLink parameter interface
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a refactoring of the MAVLink paramter interface. Solves #440.