Relaxing ROS2 topic/service field name restrictions

Back in 2015, the ROS2 rules for the names of fields in topic and services were tightened up to disallow uppercase characters (and hence camel case). The original discussion seems to have been here:

https://groups.google.com/forum/#!msg/ros-sig-ng-ros/YAyfgrvUvs0/ULthxTF51GwJ

with the follow-up PR done here:

Since then, some users of ROS2 have expressed a desire to loosen these restrictions, and put the rules back to what they were in ROS1 (allowing camel case, among other things). I’m interested in hearing opinions from the community on whether the restrictions are beneficial, harmful, or don’t matter very much. Thoughts welcome!

My 2 cents: Either use _ as the word separator throughout or CamelCase, but please not both.

Says the guy with handle Ingo_Lutkebohle :yum:

Jokes aside, from the google groups discussion I don’t really understand why the decision to enforce stricter rules in the first place was reached, but I haven’t really been following the developments as much.

In my opinion minimizing the migration effort by far outweights the consistency concerns. Consistency can be improved by having some style guide and linters to educate users and maybe even enforce the rules for new core messages. At the same time we should still allow exceptions, in particular also to accommodate non-public messages people might have. So for that reason I’m in favor to have the same rules as for ROS1, unless there is a very strong technical reason to change it (making constants easier to distinguish from fields is nice, but not a very strong reason but rather only a little bonus IMHO, especially since most IDE’s have syntax highlighting for that sort of thing).

For core messages like CameraInfo, I personally would only change them if there is a strong reason to do so and if there is consensus among a larger number of actual users of these messages that the new version is actually an improvement. For some suggestion of one person of a more consistent / understandable naming, there are probably 5 people that disagree or have their own alternative suggestions.

Says the guy with handle Ingo_Lutkebohle

Oops :wink: Having to say, that was system-generated…

Regarding migration and consistency: I agree with you there. Maybe we should have this as a warning rather than an error.

The idea is that we would stay consistent in the core (probably keeping with the current underscore approach), but would open it up so that developers of downstream packages could use what they like. My two concerns with the current restriction is for migration (as @NikolausDemel pointed out), and the “getting started” cost for new users.

We are in an interesting situation with things like CameraInfo. When we originally ported CameraInfo to ROS2, we made it follow the current underscore/lowercase restrictions. If we relax the restrictions, we could go back to making it consistent with ROS1, but at the cost of being inconsistent with earlier versions of ROS2. I’m not sure which way we would go, though it is not my intention in this thread to hash that particular detail out. I mostly just want to see what people think about the current restriction.

That would be possible as well, though I still think it is a developer unfriendly. After all, there is no reason a message can’t have CamelCase fields, so for those who want to do that they could never have a warning-free build.

I think the obvious thing to do is go with the larger current user base, so go back to the ROS 1 style. That is if consistency is the only thing that matters, but I see it as improving what was in ROS 1 to make that message more consistent with the others. By and large the rules we’re enforcing in ROS 2 were the common convention in ROS 1, it’s just that this particular message was different from the other ROS 1 messages, so when we enabled the enforcement it was out of line already.

For me the obvious case solution is to provide a linter, enable it by default, and provide a convenient way to silence the linter in particular cases that you’re unwilling to change, a la # noqa.

Why not make “case” and “_” transparent so that they may be optionally used for documentation clarity, but from a programatic perspective these would all be the same:

FredFrog
fredFrog
fredfrog
fred_frog
f_e_d_f_r_o_g
FREDfrog
etc.

take any string, toLower, substitute _ for nothing, then compare strings.

Well, the reason is consistency. All other things being equal, it would be better if there would one, consistent way of naming fields. This makes developers’ lives easier.

The trouble, of course, is that we have legacy, so the question is how to deal with that. On the one hand, ROS 2 has already made so many breaking changes that one more doesn’t seem like a big deal.

On the other hand, currently with a fairly small amount of pre-processor conditionals and appropriate “using” directives, we can write components which work on both ROS1 and ROS2, as long as the field names stay the same. If field names change, we can still address this with pre-processor conditionals, but it becomes unbearable. This is the rationale for allowing it at all.

Still, for the future, why not ensure more consistency. No harm, right? There is no advantage of either naming convention over the other, we just have to pick one.

To get rid of the warnings, we can put messages on a legacy list or something like that.

:+1: on this solution + maybe a “convention” disallowing this opt-out for new messages in core packages.

I’m still not convinced on the CameraInfo case. I do agree that consistency by itself is valuable in particular also for the core messages and maybe this is a good time to improve things, but from past experience with writing datatype converters between different systems (a few iterations of of ROS1 + X) I still believe that ease of transition in this particular case is more valuable. (But then again, I don’t really know what overhead is really incurred by changing message and field names in this case, so it’s hard to judge for me.)

Fully agreed on no harm / only benefit, if we consider only new messages and leave legacy as they are.

“Legacy list” sounds like a centrally managed solution (probably you didn’t mean that anyway), but we would need is an in-package per-message opt-out option (a la # noqa as William mentioned).

Not sure about this, sounds to me like more potential for things going wrong, as IMHO it is a surprising convention for people that are not already familiar with the rules and thus is likely to result in tools / scripts that work in many cases, but break in surprising ways since this rule was not accounted for.

Even very simple things like topic names foo/bar and /foo/bar not being treated equal has caused me many a headache in rosbag processing.