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

packet_rx_drop_count & parse_error are always 0 under /diagnostics #1285

Closed
koby08 opened this issue Jul 30, 2019 · 0 comments · Fixed by #1287
Closed

packet_rx_drop_count & parse_error are always 0 under /diagnostics #1285

koby08 opened this issue Jul 30, 2019 · 0 comments · Fixed by #1287
Labels
bug
Milestone
Version 0.33

Comments

@koby08
Copy link
Contributor

koby08 commented Jul 30, 2019

Seems like mavros tries to report parse_error and packet_rx_drop_count under /diagnostics (mavlink_diag.cpp), but their values are never incremented.

The problem seem to be in the way MAVConnInterface::parse_buffer tries to count those. It increments the m_status.parse_error, but m_status.parse_error is set to 0 in every call to mavlink_frame_char_buffer, and therefore when mavlink_diag calls get_status(), it always sees these values as 0.

MAVROS version and platform

Mavros: ?0.31.0?
ROS: ?Melodic?
Ubuntu: ?18.04?

Autopilot type and version

[ ] ArduPilot
[ X ] PX4
koby08 added a commit to koby08/mavros that referenced this issue Jul 31, 2019
@koby08
libmavconn: simplify parse_buffer, and fix dropped_packets and parse_…
836fc9f 
…error counters

Currently the dropped_packets & parse_error counters are always published as 0 in mavros_diag.cpp.
This seems to be caused by using the wrong status struct.

Seems like mavros was editing m_status after mavlink_frame_char_buffer. This struct
seems to be the parsing state, and looks like it shouldn't be modified by the caller (for example
status->parse_error is zeroed out in the end of mavlink_frame_char_buffer).
Also, the crc & signature checks done in mavros seems redundant.

r_mavlink_status seems to be the struct that holds the mavlink connection information, therefore I
changed get_status to return it instead.

This fixes mavlink#1285.
@koby08 koby08 mentioned this issue Jul 31, 2019
Merged
@vooon vooon added the bug label Aug 8, 2019
TSC21 pushed a commit that referenced this issue Oct 7, 2019
@koby08 @TSC21
libmavconn: simplify parse_buffer, and fix dropped_packets and parse_…
56d782d 
…error counters

Currently the dropped_packets & parse_error counters are always published as 0 in mavros_diag.cpp.
This seems to be caused by using the wrong status struct.

Seems like mavros was editing m_status after mavlink_frame_char_buffer. This struct
seems to be the parsing state, and looks like it shouldn't be modified by the caller (for example
status->parse_error is zeroed out in the end of mavlink_frame_char_buffer).
Also, the crc & signature checks done in mavros seems redundant.

r_mavlink_status seems to be the struct that holds the mavlink connection information, therefore I
changed get_status to return it instead.

This fixes #1285.
@vooon vooon added this to the Version 0.33 milestone Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
Version 0.33
Development

Successfully merging a pull request may close this issue.

libmavconn: simplify parse_buffer, and fix dropped_packets and parse_error counters koby08/mavros
2 participants
@vooon @koby08