Skip to content
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

VehicleInfo : add srv into sys_status plugin to request vehicule basic info #1150

Merged
merged 1 commit into from
Jan 3, 2019

Conversation

Kiwa21
Copy link
Contributor

@Kiwa21 Kiwa21 commented Jan 2, 2019

Service allow to request :

  • my target vehicle
  • a specific sysid/compid
  • all vehicles seen by mavros

Took all comment from #1147 and made a much more elegant solution.

I used an unordered_map with key & mavros_msgs::VehicleInfo directly rather than defining a new class/struc VehicleInfo and a to_msg() function like in the waypoint plugin.

Thanks !

if(req.get_all){
//Send all vehicles
for (auto &got : vehicles) {
res.vehicles.emplace_back(got.second);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps here no difference between push_back and emplace_back.
Microoptimization - add res.vehicles.reserve(vehicles.length()).

it = res.first;
}

ROS_ASSERT(it != vehicles.end());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That search & create may be moved to common function.

uint8 base_mode
uint32 custom_mode
string mode #MAV_MODE string
uint32 mode_id #MAV_MODE number
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't sure that this field is needed.

@vooon vooon added this to the Version 0.28 milestone Jan 3, 2019
@vooon
Copy link
Member

vooon commented Jan 3, 2019

LGTM, i will do some changes myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants