-
Notifications
You must be signed in to change notification settings - Fork 13
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
dvl-a50: mavlink2resthelper: fix incorrect IDs #31
base: master
Are you sure you want to change the base?
Conversation
@ES-Alexander I will work on testing this in-water over the next few weeks. |
@clydemcqueen did you ever get around to testing this? :-) |
@ES-Alexander Sadly, we did not. I'll see if I can schedule some test time w/ a DVL-rigged ROV. |
@ES-Alexander -- sorry, testing this (and PR 34) is just not making it to the top of the queue. We have limited time to run these sorts of tests on real hardware, and the test process is cumbersome (creating a docker image per PR, installing them, etc.) Are there plans to create BETA and DEV tracks for extensions? We always run the latest ArduSub BETA on production dives (to get surftrak and other nifty features). I think it would be straightforward to switch to the BETA track of this extension, if such a thing existed. |
I was prepping for a test dive later today, and I can't get this PR to run on my bench Pi running BlueOS. I'm getting errors like this:
Does this PR need to be rebased? |
Ping... any interest in this? We are doing some testing December 3-5 and could test this if you are interested. |
- `system_id` set to match the vehicle ID, since they should be part of the same system - `component_id` set to 197 (`MAV_COMP_ID_VISUAL_INERTIAL_ODOMETRY`)
b469257
to
1200a13
Compare
I rebased and it doesn't seem to have that issue when I tried running it, so hopefully able to be tested now :-)
Thanks for the ping. I think it's worth testing if it's not a hassle to do so, but given the lack of interaction with your original issue I don't expect this is a high priority for many people - mostly just a potential correctness thing, and a matter of convenience for log analysis. |
Haha, clearly not. But thanks anyway! We have 3 days on the water next week (if the weather holds), so hopefully I am able to get some of these tests done. |
system_id
set to match the vehicle ID, since they should be part of the same systemcomponent_id
set to 197 (MAV_COMP_ID_VISUAL_INERTIAL_ODOMETRY
)Fixes #30
Untested, so probably good if someone can try it before it gets merged.