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
Acoustic msg devel #21
Acoustic msg devel #21
Conversation
acoustic_msgs/README.md
Outdated
|
||
Sonar data is also orgaized into two domain categories, temporal and spatial. Messages are inteded to be intitally reported in the temporal domain by the sonar drive then converted to the spatial domain through a processing pipeline. However, some sonars report images and detections directly in the spatial domain. In this case, sensor drivers may report direclty with the appropriate temporal message. | ||
Sonar data is also organized into two domain categories, temporal and spatial. Messages are intended to be initially reported in the temporal domain by the sonar drive then converted to the spatial domain through a processing pipeline. However, some sonars report images and detections directly in the spatial domain. In this case, sensor drivers may report directly with the appropriate temporal message. |
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.
"with the appropriate spatial message"
acoustic_msgs/README.md
Outdated
* rx_angle is defined as the angle from the y-z plane at receive time. Positive rotation around the z axis. This angle defines a cone of possible returns directions for a given beam. | ||
* tx_anlge is defined as the angle from the x-y plane at transmit time. Positive rotation around the y axis. This angle defines a cone that represents the area insonified by the tx pulse. | ||
* rx_angle is defined as the elevation from the X-Z plane at receive time. This elevation is positive toward the Y axis. This elevation defines a cone of possible return directions for a given return beam. | ||
* tx_anlge is defined as the elevation from the Y-Z plane at transmit time. This elevation is positive toward the X axis. This elevation defines a cone that represents the area insonified by the tx pulse. |
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.
tx_angle
acoustic_msgs/README.md
Outdated
* All vector quantities should have plural names | ||
* data strutures should be represented as a "structures of arrays" rather than "arrays of structures". | ||
* vectors quantities may be of length zero. This shall be interpreted as unreported. (see internal MSG doccumentaiton for more detials) | ||
* data structures should be represented as a "structures of arrays" rather than "arrays of structures". |
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 no longer true -- we have beam_unit_vec[] and ping_info[], among others.
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 don't believe we have ping_info[] anywhere. I have clarified this a bit in the updated version to specify arrays of core ROS messages . Let me know what you think.
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.
Oh, sorry, I was thinking of DetectionFlag[] flags
, which is rather annoying w/r/t how much space it takes up in a rostopic echo
.
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 forgot about that. Yeah that one is kind of an edge case.
I still think we want to say something about how Structure of arrays is preferred since that's how the messages are designed.
The Detection flag it is fundamentally a uint8. We decide to make an exception for this message type since we wanted the enum to be shared for all the messages that use this field.
anyway, I took another crack at it. Take a look.
acoustic_msgs/README.md
Outdated
* data strutures should be represented as a "structures of arrays" rather than "arrays of structures". | ||
* vectors quantities may be of length zero. This shall be interpreted as unreported. (see internal MSG doccumentaiton for more detials) | ||
* data structures should be represented as a "structures of arrays" rather than "arrays of structures". | ||
* vector quantities may be of length zero. This shall be interpreted as unreported. (see internal MSG documentation for more details) |
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.
As mentioned in an email, I think this is still a bit up for discussion -- I'd prefer to force the publisher to specify sane default values, rather than forcing all consumers of the data to implement that logic.
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 see advantages to either use case. I believe this is why we chose the structure of arrays over array of structures though. That way unreported values could be of length zero to save space. Also a default value doesn't really let the user know if the value was reported or not by the sensor. I agree it adds more work for the processing pipeline though and could potentially cause unexpected bugs.
Maybe this is worth a phone discussion.
In the mean time, I have marked this as "provisional" for now though so we can complete the pull request. I think the current wording is the safer provisional rule. If we change it later, anything that supports this rule shouldn't break outright.
acoustic_msgs/README.md
Outdated
@@ -141,7 +141,7 @@ I propose the following changes from WHOI’s original message: | |||
|
|||
Multibeam and FLS sonar data (referred to as sonar data for brevity) are broken into two type categories, detections and images. Images are intended to represent Watercolumn or FLS imaging data. Detections represent a detection reported by the sonar for a given beam. | |||
|
|||
Sonar data is also organized into two domain categories, temporal and spatial. Messages are intended to be initially reported in the temporal domain by the sonar drive then converted to the spatial domain through a processing pipeline. However, some sonars report images and detections directly in the spatial domain. In this case, sensor drivers may report directly with the appropriate temporal message. | |||
Sonar data is also organized into two domain categories, temporal and spatial. Messages are intended to be initially reported in the temporal domain by the sonar drive then converted to the spatial domain through a processing pipeline. However, some sonars report images and detections directly in the spatial domain. In this case, sensor drivers may report directly with the appropriate spacial message. |
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.
spatial =)
(booo. english spelling)
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.
Fixed.
Updated documentation to match coordinate frames and explain tx/rx angles