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.

Sign up for GitHub

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

Jump to bottom

Rover: allow WATER_DEPTH mavlink message rate to be specified #28875

Merged
merged 5 commits into from
Dec 17, 2024
Merged

Rover: allow WATER_DEPTH mavlink message rate to be specified #28875

merged 5 commits into from
Dec 17, 2024

Conversation

peterbarker
Copy link
Contributor

... and reduce the default rate

this is currently unconditionally streamed at 50Hz, chewing up all available bandwidth on some telemetry radios.

@rmackay9
Copy link
Contributor

Here's the original PR that added support #17804

@rmackay9
Copy link
Contributor

Hi @peterbarker,

This looks good, what testing has been done?

@peterbarker
Copy link
Contributor Author

Hi @peterbarker,

This looks good, what testing has been done?

I found a convenient autotest, Rover.DepthFinder which seems to do all the testing required. It actually picked up the fact my original patch wasn't filtering it to send only for boat frames!

I also did a general size-check for sanity:

Board               AP_Periph  blimp  bootloader  copter  heli  iofirmware  plane  rover  sub
Durandal                       *      *           *       *                 *      -32    *
Hitec-Airspeed      *                                                                     
KakuteH7-bdshot                *      *           *       *                 *      8      *
MatekF405                      *      *           *       *                 *      8      *
Pixhawk1-1M-bdshot             *                  *       *                 *      0      *
f103-QiotekPeriph   *                                                                     
f303-Universal      *                                                                     
iomcu                                                           *                         
revo-mini                      *      *           *       *                 *      8      *
skyviper-v2450                                    *

IamPete1
IamPete1 reviewed Dec 16, 2024
View reviewed changes
Rover/GCS_Mavlink.cpp
Location loc;
IGNORE_RETURN(ahrs.get_location(loc));

for (uint8_t i=0; i<rangefinder->num_sensors(); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we cycle backends? Before this fix I have have the link be completely flooded on a vehicle with several rangefinders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should.

See last commit on here: #28877

@peterbarker peterbarker mentioned this pull request Dec 16, 2024
Merged
@peterbarker
Copy link
Contributor Author

This PR fixed the partner's problem.

IamPete1
IamPete1 reviewed Dec 16, 2024
View reviewed changes
Rover/GCS_Mavlink.cpp Outdated
Comment on lines 1276 to 1278

#if AP_RANGEFINDER_ENABLED
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Left over from a rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not in this case - I moved the send-water-depth-code to be next to the send-rangefinder-code as the #ifs already existed and the methods are related....

Removed these lines.

@peterbarker
Rover: allow WATER_DEPTH mavlink message rate to be specified
b688008 
... and reduce the default rate

this is currently unconditionally streamed at 50Hz, chewing up all available bandwidth on some telemetry radios.
@peterbarker
GCS_MAVLink: move sending of WATER_DEPTH into Rover code
87c6550 
only compiled in on Rover at the moment.

need to add an additional Rover-specific check for frame type, so move this code into Rover for now.
@peterbarker
Rover: move sending of WATER_DEPTH into Rover code
8ed2069 
only compiled in on Rover at the moment.

need to add an additional Rover-specific check for frame type, so move this code into Rover for now.
@peterbarker
Rover: only send WATER_DEPTH for boat frames
01e25f9
@peterbarker
Rover: tidy WATER_DEPTH send checks
347d5f0
@peterbarker peterbarker force-pushed the pr/water-depth-message-problem branch from f364818 to 347d5f0 Compare December 16, 2024 22:38
@peterbarker peterbarker merged commit 73710d8 into ArduPilot:master Dec 17, 2024
99 checks passed
@peterbarker peterbarker deleted the pr/water-depth-message-problem branch December 17, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IamPete1 IamPete1 IamPete1 left review comments

@rmackay9 rmackay9 rmackay9 approved these changes

Assignees
No one assigned
Labels
MergeOnCIPass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@peterbarker @rmackay9 @IamPete1