Suggestions for std_srvs

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.

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).

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

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

@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

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.

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

@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.

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

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

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.

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

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

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

@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

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

I do all my temperatures in Rankine!

I have a cautionary tale from the world of RT-Middleware, which has provided something similar to that requested since the dawn of time. I can see why it was done initially: Defining new types in RT-Middleware is quite difficult for most users. I’m sure the thinking was that providing these defaults based on common data types would make things easier for everyone. However, the result has been overwhelmingly negative. Nearly all users are using those generic data types, and instead of semantically structured data, we see things like people putting poses into arrays of floats. The result is that RT-Middleware components are nearly universally not reusable because none of them have compatible interfaces, the components are hard to understand, and debugging is a nightmare.

Do not underestimate the value of semantically typed topics and services, and type safety. You and I may use varying different names for our topics and services, but we know that if the type is the same, then we can make our nodes talk with a simple remapping. If we use generic topic and service types, then we have to read the documentation and understand how the type is being applied to be sure. The argument has been made that someone could fill in an image incorrectly, but that’s not a good argument: Such a node should be considered broken and fixed.

I think that not providing default topic and service for common data types encourages developers to think more about the interface of their node and encourages a higher level of reusability.

3 Likes

I liken the arguments here to the C versus C++ debate. I advise to check the Cpp2016 talk by Dan Saks on that topic, very enlightening.

TL;DR :
“I am used to C and it is simple to get things done. C++ is harder.”
=> C++ gives you better type checking which makes it harder to write incorrect code.

In short, yes default common message types are easy to use, but you lose the long term guarantee that a type system is supposed to give you. You surely don’t want to sending strings around even if it is easier.

So, taking the perspective of defining message types as the types we manipulate in our large distributed ROS computing systems, I would also vote for removing default common data types.

I would even go further, and remove (or do our best to avoid) implicit conversion, which initially caused lot of confusion for me :
std_msgs.Bool(true) is not a bool with value true.
It actually is a message that contain a field data.
That field data happens to be of type bool and contain the value true.
But one should not forget the field is not the message, and therefore their type are also very different.

In that same perspective I would encourage the community to find ways to improve the ROS message type definition system, so that people can rely on it because it guarantees that we write correct code, and not just merely “use” it, to “get things to work”.

Until we can get a sound type theory of messages implemented in ROS, with direct verifiable benefits for the user, and in a way that fit into the developer workflow without feeling like a hassle, there will be the temptation of “bypassing it”.

The list of potential application for a sound message typing system is long :

  • We could feasibly write test tools just based on message types, to ensure a node (or set of) is behaving as specified.
  • We could probably write provable specifications to ensure your overall system will work as we think it will, all based on message types.
  • We could write run time checker to shut down a node (and maybe restart it) as soon as it doesn’t behave as expected.
  • And much more I cant even think about right now…

We cannot do many things in ROS1. But without breaking backward compatibility, we still can :

  • emphasize the user should define his own message types, and not the std ones.
  • write test tools that verify if a node actually behave as expected when sending random data of the described type. If not it means either your node is buggy, our your message type is not defined properly. These should probably even be included in the same package as the message itself, and easy to use in some kind of basic testing…
  • actively look for ways to improve our message type system. We still have a long way to go:
>>> std_msgs.msg.Int8(2562342342342)
data: 2562342342342

BTW, Wouldnt that kind of topic deserve its own Category in Discourse ?

I agree with nearly all (maybe all?) of what @asmodehn said. The more well-defined a node’s interface is, the easier it is to do the useful things mentioned.

But, there are two potential problems. The first is that I think that there is a risk of going so far that we duplicate existing solutions (such as OMG’s IDL). Even if we fix the problems with a system such as IDL, the chance of introducing new ones of our own is high. The second is that such features would possibly be quite heavy.