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

Why must field names of messages and services be lowercase? #333

Open
Maverobot opened this issue Feb 17, 2023 · 4 comments
Open

Why must field names of messages and services be lowercase? #333

Maverobot opened this issue Feb 17, 2023 · 4 comments
Labels

Comments

@Maverobot
Copy link

In the design draft, it says

Field names must be lowercase alphanumeric characters with underscores for separating words. They must start with an alphabetic character, they must not end with an underscore and never have two consecutive underscores.

Capital letters in field names can sometimes more suitable and human-readable especially when it comes to mathematical expressions, e.g. in FrankaState. Are there special reasons why capital letters must be eliminated from the field names?

@Maverobot Maverobot changed the title Field names of messages and services must be lowercase Why must field names of messages and services be lowercase? Feb 17, 2023
@clalancette
Copy link
Contributor

The original design article went in as part of #32 . There was no rationale given for that particular rule there.

The wording of "must" in that article is in RFC2119 style, which is why it is very imperative.

In short, we'd be OK with lifting this restriction. However, we do want to keep the core using only snake_case fields (for consistency), which we would want to do with a linter. Thus, in order to lift this restriction the following would need to be done:

  1. Develop a linter for .msg/.srv/.action/.idl files that enforces some rules, and optionally enforces others.
  2. Add something to ament_lint to enable that linter to be easily called from ament.
  3. Update the .msg/.srv/.action/.idl parsers to allow uppercase letters.
  4. Update https://github.com/ros2/design/blob/gh-pages/articles/116_legacy_interface_definition.md and https://github.com/ros2/design/blob/gh-pages/articles/110_interface_definition.md to allow that style.

We aren't likely to have time to do this, so if you'd like to contribute, we'd be happy to review PRs to do the above.

@gavanderhoorn
Copy link
Contributor

I've always understood the main rationale for the no-capitals rule-of-thumb to be the fact it significantly reduces possibility for errors and makes 'guessing' the exact spelling of fieldnames much easier.

My experience with other middlewares/code generation frameworks is that, despite the fact we have extensive code-completion support these days, no-capitals is very convenient. Personally, I believe it outweighs the fact fieldnames can't follow naming of things like mathematical expressions, but this is just one voice/data point of course.

@mikaelarguedas
Copy link
Member

Some history about rationale can be found on the ROS next gen SIG. Multiple threads mention interface files but I guess this RFC sums some of it up:

Hi,

we have a pending proposal to enforce stricter rules on the contents
of Interface files (.msg and .srv files), but this RFC is specifically
about the rules for field names in interface files. Currently we allow
upper case letters in interface files, e.g. CameraInfo has fields `D`,
`K`, `R`, and `P`. We'd like to restrict field names to all lowercase
and underscores (plus some other, less controversial rules). For a
full set of the proposed rules, you can look at this work in progress
design doc:

http://design.ros2.org/articles/interface_definition.html#conventions

The advantages to enforcing these rules (and consequently changing
field names in messages like CameraInfo) is that anyone can look at a
member of a generated message struct, e.g. `CameraInfo.D`, and tell if
the member is a field or constant. It also helps us avoid name
collisions in the generated code (similar to the rule about not having
trailing `_` nor double `_` in the field name). Also, having a style
and sticking to it has an inherent value and promotes good conventions
and more readable code.

The arguments against doing the change: An Indirect consequence of
these stricter rules is that it might encourage more non-trivial
changes to field names than would otherwise occur. For example, rather
than converting the field name `D` (an actual field name in
`CameraInfo`) to `d` as a result of these stricter rules, it might be
changed instead to `distortion_vector` for clarity. This is probably a
good thing, but it will add to the disruption of migrating to ROS 2.
For the ROS 1/ROS 2 bridge that would require some manually specified
conversion rule (similar to a rosbag migration rule) to perform the
conversion automatically. However, even with ROS 1.x, it’s relatively
easy to tell whether a member of a generated message is a field or
constant based on the context.

I think I've summarized the pros and cons of the issue well, but if
I've missed something please jump in and add to the conversation.

In the short term we will be turning on the strict enforcement and
changing the fields in the messages that are affected. In the
meantime, we'd like to also solicit feedback from you about this
issue, and if we reach a different conclusion based on pros or cons we
did not come up with, then we can change our policy and potentially
the message fields again.

Cheers. 

https://groups.google.com/g/ros-sig-ng-ros/c/YAyfgrvUvs0/m/N0F5d1S5zqgJ

@clalancette
Copy link
Contributor

Thanks for the historical notes @mikaelarguedas !

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

No branches or pull requests

4 participants