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
improve GPS_* messages #48
improve GPS_* messages #48
Conversation
message_definitions/v1.0/common.xml
Outdated
<field type="int32_t" name="baseline_a_mm" units="mm">Current baseline in ECEF x or NED north component in mm.</field> | ||
<field type="int32_t" name="baseline_b_mm" units="mm">Current baseline in ECEF y or NED east component in mm.</field> | ||
<field type="int32_t" name="baseline_c_mm" units="mm">Current baseline in ECEF z or NED down component in mm.</field> | ||
<field type="uint32_t" name="accuracy">Current estimate of baseline accuracy.</field> | ||
<field type="uint32_t" name="accuracy">Current estimate of baseline accuracy (receiver dependent)</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't do better and assign this a unit of some form and ask that all receivers are nominally coerced into an approximation of the assigned unit? Stating receiver dependent on this is going to be annoying for a GCS to handle well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, and I also hate such unitless stuff....
I think I have a good idea :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
7b7de0a
to
29694c4
Compare
message_definitions/v1.0/common.xml
Outdated
@@ -3465,7 +3474,7 @@ | |||
<field type="uint16_t" name="cog" units="cdeg">Course over ground (NOT heading, but direction of movement) in degrees * 100, 0.0..359.99 degrees. If unknown, set to: UINT16_MAX</field> | |||
<field type="uint8_t" name="satellites_visible">Number of satellites visible. If unknown, set to 255</field> | |||
<field type="uint8_t" name="dgps_numch">Number of DGPS satellites</field> | |||
<field type="uint32_t" name="dgps_age">Age of DGPS info</field> | |||
<field type="uint32_t" name="dgps_age" units="cs">Age of DGPS info</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a weird unit for this, (also to late to change it now, but honestly uint32_t is almost a waste of payload space, RTK corrections are going to be worthless after 497 hours :) Could we treat it as a ms field at least just given that its a much more common unit and we have more then enough space for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll change that, but then I need to change the other PRs as well
message_definitions/v1.0/common.xml
Outdated
<field type="int32_t" name="baseline_a_mm" units="mm">Current baseline in ECEF x or NED north component in mm.</field> | ||
<field type="int32_t" name="baseline_b_mm" units="mm">Current baseline in ECEF y or NED east component in mm.</field> | ||
<field type="int32_t" name="baseline_c_mm" units="mm">Current baseline in ECEF z or NED down component in mm.</field> | ||
<field type="uint32_t" name="accuracy">Current estimate of baseline accuracy.</field> | ||
<field type="uint32_t" name="accuracy" unit="c%">Current estimate of baseline accuracy</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is c% as a unit? I'm now much more confused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
centi percent, like explained in the commit message and used in other places in the mavlink code
just like cm, but with % instead of m
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an illogical unit for the accuracy estimate of a GPS, whats 0.01% or 100% mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the units ERB produces. 100% is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How accurate is 100%? When talking about equipment that measures absolute positions anywhere from sub centimeter to 100+ levels percentages become meaningless. Especially given that you are trying to reduce this to a limited range. This proposal wastes > half the field size, and at the end of the day isn't a helpful metric to the operator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% = aprox 3cm.
I agree is almost meanigless, but is the one I get from ERB. :(
I get to see a tendence, if it is increasing is OK, if it is decreasing, I might have a problem. That is my current use of this information.
44d5ff5
to
a887b88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@WickedShell Thanks for approving this. |
No description provided.