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

[core] Get rid of confusing system hierarchy #556

Closed
duburcqa opened this issue Aug 8, 2022 · 1 comment · Fixed by #728
Closed

[core] Get rid of confusing system hierarchy #556

duburcqa opened this issue Aug 8, 2022 · 1 comment · Fixed by #728
Labels
core P0 Highest priority issue

Comments

@duburcqa
Copy link
Owner

duburcqa commented Aug 8, 2022

Currently, the sensor and controller update periods are set at engine level, while it should be specific to every robots. It would be natural to move the controller as an attribute Robot, and request the user to the setController method (currently part of EngineMultiRobot) to set it on-the-fly.

This way, the notion of "system" can be removed altogether and replaced by "robot". The engine would still be responsible for calling the controller and its internal dynamics itself, but it is already the case for any physical quantity anyway.

To avoid any conflict or confusion, getName method of Model should be removed, in favor of manually accessing name attribute of pinocchio_model. The robots themselves as no name anymore apart from the ones the engine is giving to them to distinguish between them. This mechanism is different from the sensors and motors and maybe should be refactor. It seems more rationale to identity them from the name their owner is giving them since they are basically all identical.

In the same vein, registered forces should be named. Everything should return lists ordered by motors, sensors or rotor instead of a mix of unordered dicts and list as it is currently the case.

@duburcqa duburcqa added core P0 Highest priority issue labels Aug 8, 2022
@duburcqa duburcqa changed the title [core] Find a way to introduce system options. [core] Introduce system options. Aug 18, 2022
@duburcqa
Copy link
Owner Author

duburcqa commented Sep 26, 2022

After thinking about it. I suggest requirement naming robots, in the exact same way as sensors and motors. Being able to identify uniquely a robot after getting it without having to move around a reference to the engine is very convenient. Anyway, it does make sense that each robot owns its identity. Note that the name of the robot would be different from the "name" that can be found in the pinocchio model itself. It is not a problem because it refers to the version of the mechanics. Both are relevant, the former is about the unit, the latter is about the platform.

@duburcqa duburcqa changed the title [core] Introduce system options. [core] Get rid of system notion. Sep 26, 2022
@duburcqa duburcqa changed the title [core] Get rid of system notion. [core] Get rid of the confusing system hierarchy. Sep 26, 2022
@duburcqa duburcqa changed the title [core] Get rid of the confusing system hierarchy. [core] Get rid of confusing system hierarchy. Sep 26, 2022
@duburcqa duburcqa changed the title [core] Get rid of confusing system hierarchy. [core] Get rid of confusing system hierarchy Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core P0 Highest priority issue
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant