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

Recursively load all transformer configurations. #36

Closed
wants to merge 1 commit into from
Closed

Recursively load all transformer configurations. #36

wants to merge 1 commit into from

Conversation

skasperski
Copy link

Currently configuration files are loaded recursively from all bundles, but the transformer configuration is only loaded from the most specific bundle. I would suggest to handle the transformer configuration in the same way as the orogen configs.

Not sure what the intended behavior is though. The suggested change will definitely change that behavior and probably cause issues when an previously unused transforms.rb is present in an included bundle.

@doudou
Copy link
Member

doudou commented Jan 15, 2020

This wasn't done as there is no easy way to "un-load" transformer configurations, while overloading configuration options is easy to do.

I'm however not well placed to comment on the usability issue (with or without this PR) as I don't use the script-based interface (and haven't in ages). If you know other people that could have a more informed position, would be good to mention them to get their feedback.

@skasperski
Copy link
Author

@planthaber @2maz ?

@planthaber
Copy link
Member

We were thinking about a better use of bundles, so each (base) robot gets a bundle with transformer configuration and project specific bundles (with custom scripts/models/whatever) just include the robot bundle. I think this is what bundles were intended to solve, or did in misunderstand this?

As there is (mostly) only one physical robot, the transformer config could be in the shared robot bundle and application specific bundle content could be in the project bundles.

Nevertheless, it seems like (as I understood Sebastian) the transformer config would only be loaded from the selcted bundle (the project bundle), but the goal would be to load the one of the robot bundle.

IMO we don't even need to merge the teansformer configs, but only to go up the tree if there is no config and load the one of the included bundle. Could be even better if the transformer config would have a "include_from_bundle(robotbundle)" function to be called in the Project bundle to load the config from the other bundle. This way the include would be explicit without "magic includes"

@doudou
Copy link
Member

doudou commented Feb 17, 2020

@planthaber as I said, not using the whole bundle system at all anymore ... I'm 100% using Syskit now, and am focusing on improving the usability / range of usefulness of that.

So, feel free to improve the bundle system as you see fit, to better fit your use-case.

@skasperski skasperski closed this Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants