-
Notifications
You must be signed in to change notification settings - Fork 612
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
Add multiplier value for GPS_STATUS.satellite_azimuth where 255 means 360deg #885
Add multiplier value for GPS_STATUS.satellite_azimuth where 255 means 360deg #885
Conversation
b8af2b9
to
9c38e98
Compare
9c38e98
to
04e8804
Compare
The description specifies that 255 represents 360 deg, therefore multiplier is 360/255
04e8804
to
55a79ec
Compare
Following merging of #894, this now proposes to add a multiplier value of 360/255 to be used by satellite_azimuth, rather than create a new not-a-real-unit. |
@@ -178,6 +178,7 @@ | |||
<xs:simpleType name="factor"> | |||
<xs:restriction base="xs:string"> | |||
<xs:enumeration value="1E-2"/> <!-- actual value = stated value / 100 --> | |||
<xs:enumeration value="360/255"/> <!-- actual value = stated value * 360/255, as used for GPS_STATUS.satellite_azimuth --> |
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.
@peterbarker How about
<xs:enumeration value="360/255"/> <!-- actual value = stated value * 360/255, as used for GPS_STATUS.satellite_azimuth --> | |
<xs:enumeration value="360div255"/> <!-- actual value = stated value * 360/255, as used for GPS_STATUS.satellite_azimuth --> |
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.
Or to put it another way, I have no strong opinions, but good to put this in.
degScaledToByte? degScaled255
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.
Personally, I'd prefer "360/255" because, it is both:
- directly evaluatable in pretty much any programming language for when the actual value is required for maths.
- humanly understandable as text if printed as-is (e.g. web page, Wireshark dissector, code comments, etc).
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.
Let's see what @peterbarker says - I dislike the slash sorry!
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 like the slash. It means we can continue to just eval these things.
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 prefer the slash
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.
Great!
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.
Thanks for pushing this @shancock884
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.
Marked this for DevCall to make sure tridge is good with this.
@@ -178,6 +178,7 @@ | |||
<xs:simpleType name="factor"> | |||
<xs:restriction base="xs:string"> | |||
<xs:enumeration value="1E-2"/> <!-- actual value = stated value / 100 --> | |||
<xs:enumeration value="360/255"/> <!-- actual value = stated value * 360/255, as used for GPS_STATUS.satellite_azimuth --> |
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 like the slash. It means we can continue to just eval these things.
360/256 seams more logical |
yes I made the same suggestion when raising the original issue mavlink/mavlink#2062, but in the end it seemed safer to keep the range in line with the historic description. |
Relates to mavlink/mavlink#2062, point 2.
This PR adds the "degS8" unit, and an associated comment to the mavschema.xsd file.