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

Hardware Configuration Manager #115

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

Hardware Configuration Manager #115

wants to merge 33 commits into from

Conversation

BillThePlatypus
Copy link
Contributor

This interfaces with the firmware hardware configuration manager. This adds two new services, config_set and config_get.

@BillThePlatypus
Copy link
Contributor Author

For details, see the companion request in the firmware: rosflight/rosflight_firmware#369.

Copy link
Contributor

@dpkoch dpkoch left a comment

Choose a reason for hiding this comment

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

Looking good! Thank you!

Just a few minor suggestions so far. However, it looks like your editor has indented everythin inside of the namespace rosflight_io { /* ... */ } in rosflight_io.cpp, which makes the diff impossible to read. Could you push another commit that fixes that?

Also, in config_manager.cpp, if you wrap everything in namespace mavrosflight { /* ... */ } you can get rid of the mavrosflight:: in front of all the function definitions, which would be more consistent with the rest of the code base.

rosflight/include/rosflight/mavrosflight/config_manager.h Outdated Show resolved Hide resolved
rosflight/include/rosflight/mavrosflight/config_manager.h Outdated Show resolved Hide resolved
rosflight/include/rosflight/mavrosflight/config_manager.h Outdated Show resolved Hide resolved
rosflight/include/rosflight/mavrosflight/config_manager.h Outdated Show resolved Hide resolved
rosflight/include/rosflight/mavrosflight/config_manager.h Outdated Show resolved Hide resolved
rosflight/src/mavrosflight/config_manager.cpp Outdated Show resolved Hide resolved
@BillThePlatypus BillThePlatypus changed the title [WIP] Hardware Configuration Manager Hardware Configuration Manager Mar 26, 2020
@BillThePlatypus
Copy link
Contributor Author

As a note, this also adds the config_list service, which shows available configurations.

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.

2 participants