-
Notifications
You must be signed in to change notification settings - Fork 988
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
Orientation enum name #342
Conversation
TSC21
commented
Jul 16, 2015
- Added names to orientation list;
- Added functions: one to retrieve the name when having the enum, other to retrieve the number when having the name.
distance_sensor: add usage to new sensor_orientation functions
f895673
to
3007839
Compare
3007839
to
0462cd9
Compare
/** | ||
* @brief Retrieve sensor orientation number from alias name. | ||
*/ | ||
static int orientation_from_str(std::string sensor_orientation); |
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.
Change std::string sensor_orientation
to const std::string &sensor_orientation
.
Also this function should be capable convert numeric value of enum (e.g. strtol() then check & return).
That needed for backward compatibility.
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.
Change
std::string sensor_orientation
toconst std::string &sensor_orientation
.
Why? And why not also Eigen::Quaterniond sensor_orientation_matching
?
Also this function should be capable convert numeric value of enum (e.g. strtol() then check & return).
Convert to what? MAV_SENSOR_ORIENTATION orientation
is supposed to be a number, which is used as an index to return: orientation name in case of UAS::str_sensor_orientation()
; and orientation values in case of UAS::sensor_orientation_matching()
. And orientation_from_str()
was created so we can return the orientation number/index if we give the orientation name. So I think we have all the functionalities we need.
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.
Why?
Because this reduces object copying. What you want change in UAS::sensor_orientation_matching()?
strtol()
Because there may be lots of configs with number representation. Why not just simply try to fall back to old behavior?
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.
Because this reduces object copying. What you want change in UAS::sensor_orientation_matching()?
This I undestand, but:
Because there may be lots of configs with number representation. Why not just simply try to fall back to old behavior?
... I can't. Can you code an example please?
@@ -37,7 +37,7 @@ class DistanceSensorItem { | |||
send_tf(false), | |||
sensor_id(0), | |||
field_of_view(0), | |||
orientation(-1), | |||
orientation("LOCAL_NED"), |
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 make it worse. Nope-nope-nope!
Leave orientation
as integer.
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.
But if I leave it as integer, how can https://github.com/mavlink/mavros/pull/342/files#diff-57d2f68a3e0e5cfa2db14ac20d8845ddR117 be a string???
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.
Also, this is supposed to be https://github.com/mavlink/mavros/pull/342/files#diff-2adf7423447a0e5f4880b56161efdda9R51, so how can I init as an integer and use it as a string (which it is)?!
Only one thing need to change in distance_sensor.cpp: You should pull |
Don't know what you mean... |
uint8_t sensor_id; //!< id of the sensor | ||
bool is_subscriber; //!< this item is a subscriber, else is a publisher | ||
bool send_tf; //!< defines if a transform is sent or not | ||
uint8_t sensor_id; //!< id of the sensor | ||
double field_of_view; //!< FOV of the sensor | ||
tf::Vector3 position; //!< sensor position |
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.
That's one field that need to change type: Eigen::Vector3d.
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.
OK. But please(!), instead, go to specific lines to change so to orientation handling work ok (that's the purpose of this PR).
* orientation_enum_name: distance_sensor #342: correct orientation parameter handling. lib #342: try to convert numeric value too px4_config: adapt to distance_sensor params to new features distance_sensor: restructure orientation matching and verification lib #342: Added sensor orientation string repr.
Rebased and merged. |
* master: (87 commits) package: comment-out c++03 related flags package: Replace YouCompleteMe config with @galou gist scripts: fix mavwp test: add test cases for new sensor orientation functions remove tf1 dep lib #319: Remove TF types from UAS plugin: param: new message type: ParamValue msgs: Move MAV_CMD values to separate msg plugin: command: fix build fix whitespaces in python scripts Added launch file for PX4 posix sitl to launch gcs_bridge node for bridging posix and gazebo scripts: mavftp: little speed up by aligning access to payload length launch: Add optional log_output arg distance_sensor #342: correct orientation parameter handling. lib #342: try to convert numeric value too Update iris_empty_world_offboard_ctl.launch px4_config: adapt to distance_sensor params to new features distance_sensor: restructure orientation matching and verification lib #342: Added sensor orientation string repr. test: fix prerelease building ...