search

Intermodalics / tango_ros

ROS related software for Tango
Apache License 2.0
67 stars 22 forks source link

Add missing returns for GetMap service functions #315

Closed smits closed 7 years ago

smits commented 7 years ago

Small fix, we should return something, I'm not entirely sure whether we should do this is just return true always.

smits commented 7 years ago

Based on http://wiki.ros.org/roscpp/Overview/Services#Class_Methods we should always return true IMHO.

lounick commented 7 years ago

If it always guarantees to succeed yes. In a ros service you can return false if the result could not be computed for some reason. In the call of GetMapNameFromUuid you may fail to get the string.

Actually according to TangoAreaDescriptionMetadata_get the value of the char* can be NULL if the key is not found or not set. I don't think you are allowed to create a string instance with a NULL value.

A better signature could be bool GetMapNameFromUuid(const std::string& map_uuid, std::string &map_name) and return false on any of the errors.

Sorry for the hijack :D

smits commented 7 years ago

@lounick Thanks for the Hijack, I realised the same in the meantime and implemented a refactor accordingly. PTAL @lounick and @mcopejans

lounick commented 7 years ago

@smits LGTM