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

AP_GPS: Suppress unnecessary telemetry transmission #28221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

muramura
Copy link
Contributor

If data cannot be obtained from the GPS device, telemetry transmission should be suppressed.
The GCS can detect a device malfunction if it is unable to receive telemetry from the GPS.
Since the GCS on the transmitter has a narrow bandwidth, refraining from sending meaningless telemetry data allows the bandwidth to be allocated to other telemetry.

AFTER
Screenshot from 2024-09-24 20-35-20

BEFORE
Screenshot from 2024-09-24 20-17-17

@@ -1369,6 +1369,10 @@ uint16_t AP_GPS::gps_yaw_cdeg(uint8_t instance) const

void AP_GPS::send_mavlink_gps_raw(mavlink_channel_t chan)
{
if (params[0].type == GPS_TYPE_NONE || drivers[0] == nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We continue to send this message to "clear" the GCS's concept of what the GPS data is. So it might stop displaying a position on a map or something.

That's my recollection, anyway.

OK, I looked it up: e995a19

This behaviour is on purpose.

I think this might be reasonable:

Suggested change
if (params[0].type == GPS_TYPE_NONE || drivers[0] == nullptr) {
if (params[0].type == GPS_TYPE_NONE) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with the comment.

@@ -1405,7 +1409,7 @@ void AP_GPS::send_mavlink_gps_raw(mavlink_channel_t chan)
void AP_GPS::send_mavlink_gps2_raw(mavlink_channel_t chan)
{
// always send the message if 2nd GPS is configured
if (params[1].type == GPS_TYPE_NONE) {
if (params[1].type == GPS_TYPE_NONE || drivers[1] == nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (params[1].type == GPS_TYPE_NONE || drivers[1] == nullptr) {
if (params[1].type == GPS_TYPE_NONE) {

per referenced PR, this is on purpose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will undo it.

@muramura muramura force-pushed the AP_Suppress_unnecessary_telemetry_transmission branch 2 times, most recently from fb3dcbe to 6175b63 Compare September 26, 2024 14:47
@muramura
Copy link
Contributor Author

Screenshot from 2024-09-26 23-39-44

Screenshot from 2024-09-26 23-40-44

Screenshot from 2024-09-26 23-42-43

@peterbarker
Copy link
Contributor

This PR now makes sending of gps-0 and gps-1 symmetric, which is nice.

Taking this to DevCall.

@peterbarker
Copy link
Contributor

... oh, implications are that if you set you GPS type to zero then you will no longer get messages at all, so your position will freeze at the last message, rather than starting to receive empty messages.

@muramura
Copy link
Contributor Author

muramura commented Sep 27, 2024

@peterbarker san.
If there is no GPS or GPS positioning is not possible, I think it's better not to send this message. If the GCS does not receive this message, it can know that the position has not been determined.

I think there is no point in sending this message if FIX_TYPE is less than or equal to GPS_FIX_TYPE_NO_FIX (1).
Screenshot from 2024-09-27 23-37-36

@muramura muramura force-pushed the AP_Suppress_unnecessary_telemetry_transmission branch 2 times, most recently from c71522a to 0ce307c Compare September 27, 2024 22:38
@tridge
Copy link
Contributor

tridge commented Sep 30, 2024

we need to check the impact of this change on MissionPlanner, mavproxy, plot.ardupilot.org and MAVExplorer

@peterbarker
Copy link
Contributor

Will the tools fall over if the GPS_RAW message is not present?
MissionPlanner
QGC
MAVProxy?
Mavlogdump?
plot.ardupilot.org

@tridge
Copy link
Contributor

tridge commented Oct 1, 2024

what happens if GPS1_TYPE=0 and GPS2_TYPE != 0 with MissionPlanner?

@peterbarker
Copy link
Contributor

Ping @muramura can you test the tooling, please?

@EosBandi
Copy link
Contributor

EosBandi commented Oct 1, 2024

Mission Planner uses the data from GLOBAL_POSITION_INT messages for displaying vehicle location, if no GLOBAL_POSITION_INT messages are received at all or it contains zero lat/lng then it uses GPS_RAW_INT for location data. GPS2_RAW messages are not used to display location.

@muramura muramura force-pushed the AP_Suppress_unnecessary_telemetry_transmission branch from 0ce307c to ddd6e62 Compare October 2, 2024 14:51
@muramura
Copy link
Contributor Author

muramura commented Oct 2, 2024

@peterbarker san

MISSION PLANNER recognizes GPS2.

gps2 gps2-1

@EosBandi
Copy link
Contributor

EosBandi commented Oct 2, 2024

@muramura Indeed, however it wont use it for displaying position on the map. As described above.

@muramura
Copy link
Contributor Author

muramura commented Oct 3, 2024

@EosBandi san

The position of GPS2 is reflected in the location data of GLOBAL_POSITION_INT.

gps2-112 gps2-113

@EosBandi
Copy link
Contributor

EosBandi commented Oct 3, 2024

@muramura san, you are right and this is a feat of ArduPilot.
The important thing is that your proposed change does not impact Mission Planner.

@muramura muramura force-pushed the AP_Suppress_unnecessary_telemetry_transmission branch from ddd6e62 to 2b4cc13 Compare October 4, 2024 14:50
@muramura
Copy link
Contributor Author

muramura commented Oct 5, 2024

APM PLANNER2 is OK.

Screenshot from 2024-10-05 15-21-24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants