Suggestions for std_srvs

1

Hi,

I think std_srvs should be extended to include services required for common data types like below.

(Get bool service)
std_srvs/GetBool

bool result

(Set string service)
std_srvs/SetString
string data

bool success
string message

(Get string service)
std_srvs/GetString

string result

(Set int service)
std_srvs/SetInt32
int32 data

bool success
string message

(Get int service)
std_srvs/GetInt32

int32 result

(Set float service)
std_srvs/SetFloat32
float32 data

bool success
string message

(Get float service)
std_srvs/GetFloat32

float32 result

Probably it’s good to have getter and setter service for all primitive data types in std_msgs but I think above would cover the most of the common requirements

Also Trigger service can be considered as a GetBool service but I think it’s more clearer to have separate service for the perpose of looking up an value.

2 Likes
2

I agree something like that would be very useful. I’ve used a package with a bunch of similar definitions myself.

I am curious about the distinction between get and set. You added bool success; string message to the setters but not the getters. Why do you think one should have them but not the other? Personally I’d lean to not having them at all. You can already report failure anyway in your service handler.

3

:+1:

I have implemented StringTrigger.srv a couple times myself.

4

Hi,

I did not included string message thinking the same way as you that they are probably not that required. But if it is required for consistency, I’m open to have string message (and probably bool success as well) for conveying error message/failure of result in the return message.

5

But you did include them on the setters, right? I think they’re not needed for those either, since all services can return an error (including a message) already by throwing an exception in the service handler (in C++ you can also return false, but then you can’t set a message).

6

@dirk-thomas is the lord of this particular kingdom. Would he be likely to accept a pull request for updates similar to these?

7

This suggestion pops up once in a while, so there are many users stumbling about this. These services used to be included in std_srvs once upon a time but were intentionally removed in 2012:

Ken Conley wrote back then:

one of the reasons we purged so many services is that its
hard to have semantically meaningful common services. Services are
more equivalent to method declarations, so a ‘GetBool’ service is
similar to naming a method or interface, ‘GetBool’. There are cases
where such a method is reasonable, but it doesn’t provide any higher
level semantic other than the type signature.

http://lists.ros.org/lurker/message/20120201.115037.1eb65481.en.html

3 Likes
8

@Martin_Guenther already summarized the rational why those generic services don’t exist anymore. I agree with that and don’t think these services should be added. Creating custom services with semantic meaning should be preferred.

1 Like
9

I can see that point, but then why do we have generic messages in std_msgs? I find those far less useful than a generic SetBool or a SetInt32.

1 Like
10

The majority of messages in std_msgs are mostly there for legacy reasons. We would have purged them when we purged std_srvs, however we judged that although the’re not recommended, they are used in a lot of places and the disruption of removing them will cost more than maintaining the package in it’s current state.

We could make the package description more proactive, but the wiki page is pretty clear:

Package description.

Standard ROS Messages including common message types representing primitive data types and other basic message constructs, such as multiarrays. For common, generic robot-specific message types, please see common_msgs.

Wiki Overview:

std_msgs contains wrappers for ROS primitive types, which are documented in the msg specification. It also contains the Empty type, which is useful for sending an empty signal. However, these types do not convey semantic meaning about their contents: every message simply has a field called “data”. Therefore, while the messages in this package can be useful for quick prototyping, they are not intended for “long-term” usage. For ease of documentation and collaboration, we recommend that existing messages be used, or new messages created, that provide meaningful field name(s).

Note that this package also contains the “MultiArray” types, which can be useful for storing sensor data. However, the same caveat applies: it’s usually “better” (in the sense of making the code easier to understand, etc.) when developers use or create non-generic message types.

1 Like
11

@Martin_Guenther, @dirk-Thomas, @tfoote

Thanks for your valuable insights on rationale behind stripping down std_srvs. I also was in understanding that defining common services may not serve much purpose other than probably preventing couple of very similar looking imports from couple of packages sometimes.

So I think we are in agreement here that Those should be more semantically defined like.
[mypackage1_srvs/GetAwesomeVariable]

bool value

etc.

Or in fact if somebody find that his organization probably have lot of such use cases and probably should use a common service signatures is it a good idea to define a company(organization) wide package for the purpose.?
[myorganzation_srvs/GetBool]

bool value

etc.

12

As with std_msgs we have learned from experience that it’s better to have semantic meaning. I would strongly recommend against using myorganization_srvs/GetBool for the same reasons that we discourage it in std_srvs.

The example of mypackage1_srvs/GetAwesomeVariable has semantic meaning such that if you do a rosservice list you know that this service represents AwesomeVariable.

As an example, if you have another service which is mypackage1_srvs/GetStatus which simply returns an int.

The status might be {0: OK, 1: ERROR, 2: UNINITIALIZED} whereas the Awesome Variable represents a real value such as what floor of the building you’re on.

They both carry the same data structure, and have the same fields. But the data carries different meanings.
If you were to remap the Status service topic to the AwesomeVariable topic suddenly your robot would be happy on the ground floor, but as soon as you moved to the 1st floor it would start erroring and say you went down to the basement, suddenly your status would be negative which is undefined behavior. Since the data is not semantically interchangeable it should be represented as a different type.

1 Like
13

But is it really the job of the ROS topic type to carry meaning? Why not use the advertised name for that?

Don’t get me wrong, I think it is often very helpful that topics and services carry semantics as you described it. It makes something easier to understand and less error prone if used correctly. But on the other hand, it requires to create a bunch of services or topics for things which are in many cases the same or very similar. I sometimes find myself “missusing” topics just to not have to introduce my own topics just for convenience.

This is especially important when thinking about one of the main strong points of ROS (in my opinion): The strong emphasis on interconnecting different parts of an application together without having to couple them specifically. When I use newly defined topics and services for that, I loose some of the flexibility I otherwise have when using just the generic types of std_msgs. Instead of having two packages perfectly on their own ( of course they are not on their own as they referr to std_msgs or std_srvs but as they will be present on every ros system it is basically the same as having no specific dependency), I will need to either have one as the dependency of the other or create a third, just to define the communication type.

Additionally there is the option of using the advertised name for semantics. Simple example would be the camera image of a stereo camera simply named camera_left and camera_right instead of using a different message type for each.

2 Likes
14

I wonder here if the confusion is because it sounds like it is less about semantic meaning and more about a kind of type safety. Tully’s story is like this - making absolutely sure you can’t execute if you call the wrong thing. Georg’s statement is also true - the advertised name is just as much a helper in guiding you in correct wiring but it doesn’t give you type safety and is probably more closely aligned to what people think of as semantics than the type name itself.

For some cases, like the image example, the line between relying on type name vs topic/service name is neither black or white. It is definitely half of one and half of the other.

15

Yes indeed it’s always a judgement call on a case by case basis.

But I’d like to highlight that there are two dimensions of semantic meaning encapsulated in the type and topic names. There is the semantic meaning of the type and the semantics of what is represented in the instance of the topic.

What I’ve been discussion above has been about the semantic meaning of the type. Such that anything with the same semantic meaning should be interchangeable. This is Image vs PointCloud (both of which have used std_msgs MultiArray datatypes in the past in ROS), or simpler RelativeHumidity vs Illuminance

These messages have very similar data structure, but have very different meanings. It is an error to feed an illuminance signal into something that is processing the humidity.

The second level mentioned is the semantic meaning of the instance which we recommend be captured by the topic name. /left_camera/image vs /right_camera/image These are both images with the same type and can both be run through any image processing pipeline. And you can still get bad behavior by feeding left camera data to both sides of a stereo module. But the system knows that it’s invalid to feed a PointCloud into the input of a stereo image.

I agree that interconnectability is one of the values that ROS provides. But this value is actually provided by not taking the shortcuts you mentioned and “missusing” messages. By having the carefully defined datatypes that are fully self contained and have semantic meaning, a modules interface can be defined basically by the types and you know that if you have a matching typed publisher you can hook it up and it will process the data.

I’ve seen many systems where if you use loosely defined datatypes, or missuse defined datatypes, you get modules that theoretically have the same information but they’ve diverged in their representation and can no longer function correctly due to the diverged use of the message. Which then breaks the whole introspection system as well as the interface definitions.

3 Likes
16

I agree, that missusing messages and services is a tempting thing when prototyping software and it often doesn’t get replaced later on.

However, I like to see those very simple services kind of the same as c++ functions. If you take the setBool service for example. You would create a service called e.g. set_detection_enabled of type SetBool

In c++ you would create a function called set_detection_enabled which takes a bool as an argument.

Now you could create a custom service type, let’s name it SetDetectionEnabledStatus which has a boolean field called is_enabled. This is how I understand the counter-general-services position above.

Taken back to c++ terms I understand this as a typdef from bool to detection_enabled_status which doesn’t feel quite right to me.

As soon as I would pack something into a struct or class or enum in c++ I agree, that it does make sense to also define it’s own message type. This also fits together with Tully’s example that you could in theory use an image for a pointcloud, but you wouldn’t.

Another point is that everybody knows how to use these standard types. If we go back to my example and let’s say, we created a service called SetDetectionEnabledStatus, it’s not clear from the type which states are actually allowed. It could also contain an enum field with 20 different states. If it’s just of type SetBool, then everybody knows the two possible values just by looking at the type.

I am very well aware that the SetBool service does exist in std_srvs and I’v been cloning the repo to my workspaces a couple of times before it was released as package, however I think that the same argumentation goes for something like setFloat, setInt and so on. To quote a colleague: A float is a float, is a float. If you decide to call a service called set_distance with a measurement received from a humidity sensor or from a random number generator, that’s ok. I think it is quite obvious, that this probably won’t produce the desired behavior, however.

TLDR: I would prefer to have predefined set and trigger services with basic datatypes, however I see the risk of tempting people to kind of missuse datastructures out of pure lazyness and that this should be prevented as good as reasonable (reasonable as in the bool typedef example).

1 Like
17

There are some predefined services already:

From the list proposed, I could see enough grey area to allow GetBool, but the others I don’t think are good ideas. With bool, as @fmauch pointed out, you know what the valid inputs are (true and false). If only one of those, or neither, were valid, then you would use Empty or Trigger instead.

With the get/set of the other types, I think the possible values are not clear and would benefit from a more specific type. String is on the fence for me, but int and float I feel pretty strongly should have constants or units attached to them by using a custom Service type with comments. For example, using SetFloat32 for a service called /set_desired_temp is the kind of abuse of these types we want to avoid. In the case of string, I could see cases where using just a set/get string service type would be ok, but most all of them would benefit from, if not require, further clarification in a custom service type.

1 Like
18

@tfoote you make compelling arguments :wink:

In general I am convinced by the logic of having it semantically sound and using exchange formats in a well defined manner, but on the other hand I also agree with @fmauch. When we treat services as function calls (or somewhat remote procedure calls), why do we treat types different than in such a function call?

In Particular, @wjwwood, what exactly would be the abuse of using a SetFloat32 fot /set_desired_temp? That no unit is given? Or that you could give values that are excedingly large or something like that? I think I would use a simple float as parameter for this if implemented as method. Thats why there are return values :slight_smile:

Aggain, for any more complex data type, where I would use a struct or class, I agree, custom services are the thing to use. But for simple data types? Where is the harm in someone inputting something semantically differnt in the service call if it is syntactically correct? Thats why I asked about if it really should be the task of the type definition to handle this. Because if i wanted to, I could also write some weird stuff into custom defined types. For example an image could very well be wrongly encoded or contain weird depth data. When something is offering to accept floats, the input type is also quite well defined as beeing a float.

By the way, thanks everone for this discussion, we had simmilar discussions offline, so getting the insigths and design views of everybody is quite helpfull.

1 Like
19

Exactly that. Is the desired temperature in Celsius or Fahrenheit or Kelvin? You could put this information into a topic name, but then how would a general purpose tool (say a node that visualized the temperature on a thermometer) know which units you are using? Extracting it from the topic name is hard and likely error prone. If it is agreed on in the message definition (either by comment saying it is always Celsius or by having another field that lets the publisher say which units it is in) then it is always clear to both the developer and any program.

1 Like
20

I do all my temperatures in Rankine!