esc_report: change esc_temperature field to allow negative values [needs mavlink submodule update] #18108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Trust no one, but let's default to the spec and handle special cases if necessary. |
dagar
reviewed
Aug 19, 2021
msg/esc_report.msg
Outdated
| @@ -3,7 +3,7 @@ uint32 esc_errorcount # Number of reported errors by ESC - if supported | |||
| int32 esc_rpm # Motor RPM, negative for reverse rotation [RPM] - if supported | |||
| float32 esc_voltage # Voltage measured from current ESC [V] - if supported | |||
| float32 esc_current # Current measured from current ESC [A] - if supported | |||
| uint8 esc_temperature # Temperature measured from current ESC [degC] - if supported | |||
| int16 esc_temperature # Temperature measured from current ESC [centiDegC] - if supported | |||
There was a problem hiding this comment.
Can we keep this SI at the uORB level? Using a float32 is fine unless you think int8 is sufficient (127 C max might not be quite enough for ESCs).
cmic0
force-pushed
the
pr-esc-negative-temp
branch
2 times, most recently
from
bfb8fb5
to
fb4d859
Compare
|
mavlink/mavlink#1663 has been merged, i've updated this PR to use degC @dagar |
cmic0
commented
Aug 31, 2021
Signed-off-by: Claudio Micheli <claudio@auterion.com>
cmic0
force-pushed
the
pr-esc-negative-temp
branch
from
fb4d859
to
f4c70a9
Compare
cmic0
changed the title
bkueng
approved these changes
Sep 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Title is self explanatory. This PR needs to go in after the relative mavlink message is updated, mavlink/mavlink#1663.
@dagar i've also noticed that the UAVCAN spec defines the temperature in Kelvin for
uavcan.equipment.esc.Status(https://legacy.uavcan.org/Specification/7._List_of_standard_data_types/), looking at the code we've always filled the esc_temperature directly from the UAVCAN message.. not sure who to trust there :|