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

[FEATURE]: Remove unnecessary modifications to MAVROS and source build #312

Open
evan-palmer opened this issue Sep 22, 2024 · 5 comments · May be fixed by #324
Open

[FEATURE]: Remove unnecessary modifications to MAVROS and source build #312

evan-palmer opened this issue Sep 22, 2024 · 5 comments · May be fixed by #324
Labels
enhancement New feature or request needs triage This issue needs triage support

Comments

@evan-palmer
Copy link
Collaborator

Feature Type

Changing existing functionality in the BlueROV2 driver

Problem Description

The Dockerfile currently makes a few manual changes to the mavros_extras/CMakelists.txt file to address some linking errors to yaml-cpp. A PR was recently merged to address this issue (and subsequently updated in mavros!1994).

Feature Description

Remove stale changes in the Dockerfile.

Alternative Solutions

N/A

Additional Context

No response

@evan-palmer evan-palmer added enhancement New feature or request needs triage This issue needs triage support labels Sep 22, 2024
@amarburg
Copy link
Collaborator

I've started an MR on this, but am seeing some unrelated (?) issues due to ... upstream API changes?

15.35 /home/ubuntu/ws_blue/src/auv_controllers/velocity_controllers/src/integral_sliding_mode_controller.cpp: In member function ‘virtual controller_interface::return_type v
elocity_controllers::IntegralSlidingModeController::update_and_write_commands(const rclcpp::Time&, const rclcpp::Duration&)’:
15.35 /home/ubuntu/ws_blue/src/auv_controllers/velocity_controllers/src/integral_sliding_mode_controller.cpp:345:15: error: ‘struct hydrodynamics::Inertia’ has no member nam
ed ‘getMassMatrix’; did you mean ‘mass_matrix’?
15.35   345 |     inertia_->getMassMatrix() * (proportional_gain_ * velocity_error) +
15.35       |               ^~~~~~~~~~~~~
15.35       |               mass_matrix
15.35 /home/ubuntu/ws_blue/src/auv_controllers/velocity_controllers/src/integral_sliding_mode_controller.cpp:346:16: error: ‘struct hydrodynamics::Coriolis’ has no member na
med ‘calculateCoriolisMatrix’; did you mean ‘calculate_coriolis_matrix’?
15.35   346 |     coriolis_->calculateCoriolisMatrix(velocity_state) * velocity_state +
15.35       |                ^~~~~~~~~~~~~~~~~~~~~~~
15.35       |                calculate_coriolis_matrix
15.35 /home/ubuntu/ws_blue/src/auv_controllers/velocity_controllers/src/integral_sliding_mode_controller.cpp:347:15: error: ‘struct hydrodynamics::Damping’ has no member nam
ed ‘calculateDampingMatrix’; did you mean ‘calculate_damping_matrix’?
15.35   347 |     damping_->calculateDampingMatrix(velocity_state) * velocity_state +
15.35       |               ^~~~~~~~~~~~~~~~~~~~~~
15.35       |               calculate_damping_matrix
15.35 /home/ubuntu/ws_blue/src/auv_controllers/velocity_controllers/src/integral_sliding_mode_controller.cpp:348:24: error: ‘struct hydrodynamics::RestoringForces’ has no me
mber named ‘calculateRestoringForcesVector’; did you mean ‘calculate_restoring_forces_vector’?
15.35   348 |     restoring_forces_->calculateRestoringForcesVector(system_rotation_.readFromRT()->toRotationMatrix());
15.35       |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
15.35       |                        calculate_restoring_forces_vector

I can beat on it a bit.

Also, I am ... confused by the <pkg>-gbp-release process. What's required to use a more modern mavlink and eliminate the matching monkey-patching to that package?

@evan-palmer
Copy link
Collaborator Author

unrelated (?) issues due to ... upstream API changes?

Yeah, I made some changes to that package and haven't merged auv_controllers!41 to resolve that yet. I was going to work through your PR there first.

Also, I am ... confused by the -gbp-release process. What's required to use a more modern mavlink and eliminate the matching monkey-patching to that package?

I'm probably going to submit a PR soon to fix this. I think the only change needed there is to update the mavros/dependencies.rosinstall file to a version that includes that fix.

@amarburg
Copy link
Collaborator

I'm probably going to submit a PR soon to fix this. I think the only change needed there is to update the mavros/dependencies.rosinstall file to a version that includes that fix.

Again, not an expert, so I may be misreading it, but it looks like all of the release versions in that repo are quite old (and don't have that fix) e.g. the "rolling" release:

https://github.com/mavlink/mavlink-gbp-release/blob/release/rolling/mavlink/pymavlink/generator/mavgen.py

Will this repo get forward-ported (automatically?) with a new release of mavros?

@evan-palmer
Copy link
Collaborator Author

auv_controllers should be updated and working for Rolling, Jazzy, Iron, and Humble now.

Will this repo get forward-ported (automatically?) with a new release of mavros?

I think that Vladimir has to do that manually... not sure though - I need to ask

@evan-palmer
Copy link
Collaborator Author

Here is the relevant MAVROS issue that I've submitted: mavros!1995

@amarburg amarburg linked a pull request Sep 26, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs triage This issue needs triage support
Projects
None yet
2 participants