Update message documentation style #119
Merged
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
Signed-off-by: Louise Poubel <[email protected]>
tfoote
reviewed
Dec 16, 2021
| /// \brief Data collected from temperature sensor. Unit: Celsius | ||
| float temperature_ = 29; | ||
|
|
||
| /// \brief Data collected from salinity sensor. Unit: PSU |
There was a problem hiding this comment.
Suggested change
| /// \brief Data collected from salinity sensor. Unit: PSU | |
| /// \brief Data collected from salinity sensor. Unit: PSU (Practical Salinity Unit or g/kg) |
| /// \brief \TODO(chapulina) | ||
| ignition.msgs.Vector3d posRPH_ = 20; | ||
|
|
||
| /// \brief \TODO(chapulina) |
There was a problem hiding this comment.
Suggested change
| /// \brief \TODO(chapulina) | |
| /// \brief Velocity wrt to ground \TODO(chapulina) check axes |
There was a problem hiding this comment.
I removed these on purpose, I'm adding the clearer explanation on https://github.com/osrf/lrauv/pull/81/files#diff-638da727e840523f41a6bdea345acc02e4ab2b12ee06b88aa70ac15ae8b07654.
| /// \brief \TODO(chapulina) | ||
| ignition.msgs.Vector3d posDot_ = 21; | ||
|
|
||
| /// \brief \TODO(chapulina) |
There was a problem hiding this comment.
Suggested change
| /// \brief \TODO(chapulina) | |
| /// \brief Water velocity \TODO(chapulina) check axes etc |
| /// \brief \TODO(chapulina) | ||
| ignition.msgs.Vector3d rateUVW_ = 22; | ||
|
|
||
| /// \brief \TODO(chapulina) |
There was a problem hiding this comment.
Suggested change
| /// \brief \TODO(chapulina) | |
| /// \brief Roll pitch and yaw rates \TODO(chapulina) check axes etc. |
| /// \brief Duplicate of posRPH_ | ||
| ignition.msgs.Vector3d rph_ = 13; | ||
|
|
||
| /// \brief TODO(chapulina) |
There was a problem hiding this comment.
Suggested change
| /// \brief TODO(chapulina) | |
| /// \brief World frame velocity in m/s TODO(chapulina) check axes etc |
| /// neutral volume push the vehicle upwards. | ||
| float buoyancyPosition_ = 11; | ||
|
|
||
| /// \brief Vertical position of the vehicle with respect to sea level. Higher |
There was a problem hiding this comment.
Suggested change
| /// \brief Vertical position of the vehicle with respect to sea level. Higher | |
| /// \brief Vertical position of the vehicle with respect to mean sea level. Higher |
Sea level is ambiguous with tides. There's MSL or there's depth to current surface, or several other semi-obscure options. This could be alternatively switched to a TODO.
|
Ouch, I merged before your review, @tfoote . I'll be sure to capture your suggestions in a new PR. |
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.
The largest part of the diff is just updating the documentation style to use doxygen, even if we're not actually generating docs for this project, that makes it easier to read and maintain.
Also made it explicit which fields are not used or populated. The idea is that we update those as we start using them.
The fields that I'm planning on updating with new reference frames have
TODO(chapulina).