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

Support placing message files in subfolders of "msg" #213

Open
firesurfer opened this issue Apr 11, 2017 · 12 comments
Open

Support placing message files in subfolders of "msg" #213

firesurfer opened this issue Apr 11, 2017 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@firesurfer
Copy link

Hi,
I wanted to structure my messages in subfolders like this:

package
    |-CMakeLists.txt
    |-msg
            |-message1.msg
            |-subfolder1
                 |-message2.msg

My CMakeLists for generating the messages looks like this:

file(GLOB_RECURSE MESSAGE_FILES msg
    "*.msg"
)

rosidl_generate_interfaces(${PROJECT_NAME}
  ${MESSAGE_FILES}
  DEPENDENCIES builtin_interfaces ros2_components_msg
)

When calling ament build it fails with the error message:
Interface file with unknown parent folder:

@dirk-thomas
Copy link
Member

This is not a supported use case atm. The messages need to be in the subfolder msg and services in srv.

@firesurfer
Copy link
Author

What would it take to support this case? Is it planned to support it in the future?

@dirk-thomas
Copy link
Member

We are not planning to add support for the use case atm. The message generation code would need to be updated to allow that. Currently it relies on the folders to be msg / srv. The logic to decide which interface is a message and which is a message derived from service is very simple and doesn't require passing additional context information. Also it implicitly ensures that message names or service names are unique.

If PRs extend the CMake / Python interfaces involved in the message generation process we would certainly consider them to be integrated. But it should be made sure that this use case doesn't complicate each generator too much (trading features for code complexity).

@dirk-thomas dirk-thomas added the enhancement New feature or request label Apr 11, 2017
@dirk-thomas dirk-thomas changed the title Generation fails if message is in subsubfolder Support placing message files in subfolders of "msg" Apr 11, 2017
@firesurfer
Copy link
Author

I will have a look at the message generation process. My idea is to treat messages in a subfolder of msg as messages. And services in a subfolder of srv as services. I don't think this would affect the uniqueness of the message names.

Only thing I'm wondering about is why the generation process doesn't use the fileextension to distinguish between messages and services.

@mikaelarguedas
Copy link
Member

Only thing I'm wondering about is why the generation process doesn't use the fileextension to distinguish between messages and services.

It does use extensions to differenciate them. But each service generate a Request and a Response message that have an msg extension. That's where uniqueness of message name need to be ensured

@dirk-thomas
Copy link
Member

I will have a look at the message generation process. My idea is to treat messages in a subfolder of msg as messages. And services in a subfolder of srv as services. I don't think this would affect the uniqueness of the message names.

A user can easily create two .msg files with the same name than which will conflict after message generation:

  • msg/foo/MyName.msg
  • msg/bar/MyName.msg

Only thing I'm wondering about is why the generation process doesn't use the fileextension to distinguish between messages and services.

Because a service (e.g. Foo.srv) is expanded into two message definitions (FooRequest.msg and FooResponse.msg) but these .msg files still belong into the srv folder / namespace.

@wjwwood
Copy link
Member

wjwwood commented Apr 26, 2017

We just discussed this in an issue triage meeting, and our team's consensus is that we will not spend time on this feature because of the issues @dirk-thomas pointed out above.

If you really want this feature, I'd suggest trying to justify why you need it and offer to open a pull request which addresses the concerns with respect to the potential collision of message files since the they don't share a folder.

@firesurfer
Copy link
Author

Perhaps you could give me a recommendation/alternative how to handle a package with a large number of message files? I thought the best way to do this is to put the message files into subfolders. Unfortunatly I got the requirement to put all messages into the same package.

@wjwwood
Copy link
Member

wjwwood commented Apr 26, 2017

@firesurfer why can you not just put all the message files in the same folder?

I don't understand why that makes it easier to organize the code, not because I don't understand the hierarchy is useful for organizing, but because the folders would not affect the C++ header layout or the C++ namespaces. So you'd end up with message files in folders but you code (and others code) still have to use them from a flat namespace.

@firesurfer
Copy link
Author

Of course I can put all messages files in the same folder. It would work without any problems (That's what I'm actually doing at the moment). I just wanted to bring some structure into the messages on filesystem level, because over the time there will be more components added to the overall project and these components will need new message. So my thought was I could put them into subfolders.

I actually do not care if they are put into extra namespaces or not. I will just have to easier to find the in the filesystem.

@wjwwood
Copy link
Member

wjwwood commented May 3, 2017

I just wanted to bring some structure into the messages on filesystem level, because over the time there will be more components added to the overall project and these components will need new message. So my thought was I could put them into subfolders.

I think that the position of the people at OSRF is that having a folder structure different from the namespace is not very useful unless it is matched in the generated code. Also, that the constraint of all message files for package needing to be in the same folder is useful, because it implicitly prevents messages with duplicate names.

However, if you see value in having additional hierarchy in the folder layout then we shouldn't stop you from doing it. So, I think we would take a pull request to allow that, but it needs to check for collisions manually, i.e. that you don't end up with something like this:

my_pkg
└── msg
    ├── arm
    │   └── Position.msg
    └── base
        └── Position.msg

Which if the constraint is relaxed would not be caught by any checks in our code currently.

@void-robotics
Copy link

Hello,

I'd also like to note that I agree with @firesurfer that it'd be useful to be able to put the srv, msg, and action folders in another folder so that repo organization is better.

I understand the concern of conflicting names, but if this is a concern, I'd vote to have this be a compile-time error so that this feature can be realized, as I think it will not only help large codebases stay better organized but also novice coders understand that messages, services, and actions are all similar concepts.

I also understand that this is probably low priority in ROS2, as it doesn't seem crucial; but I'd like it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants