Proposer: Blaise Gassend
Present at review:
- List reviewers
Question / concerns / comments
Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.
- Changing the default value for parameter "frame_id" from "base_laser" to "FRAMEID_LASER" (staying consistent with hokuyo node).
- BG: Changing to "laser" for both hokuyo and sick.
- (Agreeing with John). Yes, the default frame_id should be the same for both the hokuyo node and sick
- BG: Done. Changed to "laser".
There should be an explanation of how ~frame_id relates to ~inverted
- BG: Updated the definition of the parameter.
- API itself seems fine.
- I'm 90% sure that inverted does not change the scan direction, only the direction in which the scans are output.
Some more documentation on frameids and invertedness would be helpful, but this is all technically specified by sensor_msgs/LaserScan message. Angles are measured counter-clockwise with Z up and X forward. As such, switching inverted should change the data ordering, swap the angle_min and angle_max, negate the angle_increment and the time_increment, and change the timestamp to be that of the "last" scan (now first in the range array).
Blaise: Just checked the code. Setting inverted just swaps angle_max and angle_min and negates the angle_increment. I think will actually have the desired result if your laser happens to be mounted upside-down compared with the z-axis. However, looking at sensor_msgs/LaserScan I find the wording unclear. When I read "in frame frame_id, laser is assumed to spin around the positive Z axis (counterclockwise, if Z is up) with zero angle being forward along the x axis" this tells me that if the Z axis is set, the direction of rotation of the laser is also set (but the direction in which angles are measured is not). So when you set the inverted flag, you are actually working with a laser that is spinning backward, in violation of the spec. It would make more sense in my opinion to say that "in frame frame_id, angles are measured around the positive Z axis (counterclockwise, if Z is up) with zero angle being forward along the x axis". This way you are defining how angles are measured, but not which way the laser needs to spin. I guess the big underlying question is: are negative angle increments legal?
To be filled out by proposer based on comments gathered during API review period
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved