add support for hardware trigger configuration (Gen2 DVL) - #38
Open
wltry wants to merge 4 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new ROS2 configuration parameter to control Gen2 DVL hardware triggering, while preserving compatibility with Gen1 devices by only sending the setting when the DVL reports support for it.
Changes:
- Add
hardware_trigger_enabledto the DVL configuration model and JSON (de)serialization (optional for Gen2). - Update the ROS2 driver to read the current DVL configuration before applying startup parameters, and to ignore
hardware_trigger_enabledwhen unsupported. - Extend configuration defaults/docs and JSON parsing tests to include the new field.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
waterlinked_dvl_driver/src/waterlinked_dvl_driver.cpp |
Reads current config before applying ROS parameters; conditionally applies hardware trigger. |
waterlinked_dvl_driver/src/waterlinked_dvl_driver_parameters.yaml |
Declares new ROS parameter hardware_trigger_enabled. |
waterlinked_dvl_driver/config/dvl.yaml |
Adds default value and user-facing comment for the new parameter. |
libwaterlinked/test/test_json.cpp |
Updates JSON test payloads and assertions for the new config field. |
libwaterlinked/src/protocol.cpp |
Adds optional parsing/serialization for hardware_trigger_enabled. |
libwaterlinked/include/libwaterlinked/protocol.hpp |
Adds hardware_trigger_enabled to the Configuration struct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or 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
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.
Changes Made
Add hardware_trigger_enabled configuration parameter, for DVL Gen2 hardware.
The driver checks the current configuration to keep setting configuration compatible with Gen1 DVL.
Associated Issues
Testing
Tested that the parameter is ignored on A50 DVL (v2.7.2 fw). Tested that setting hardware_trigger_enabled=true for A250 is reflected as ROS2 parameter into the A250 configuration.