Add consistent transitions for controllers in rqt_controller_manager #2163
Conversation
github-actions
bot
requested review from
anfemosa,
arne48,
bijoua29,
bmagyar,
christophfroehlich,
erickisos and
peterdavidfagan
|
let me know how to proceed with this |
| action_activate = menu.addAction(self._icons["active"], "Activate") | ||
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Cleanup") | ||
| action_activate = menu.addAction(self._icons["active"], "Activate (active)") | ||
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Cleanup (unconfigured)") |
There was a problem hiding this comment.
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Cleanup (unconfigured)") | |
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Unload and load (unconfigured)") |
As I've mentioned in the issue, I don't have a strong opinion on this "Unload and load" or "cleanup", whether to keep it or remove it.
If @christophebedard are ok with it, fine for me
There was a problem hiding this comment.
I am not of a too strong opinion on it atm either, I think getting it is not of extreme importance to the issue in question and we can add it later, I'll go for whatever @christophfroehlich is ok with as well
There was a problem hiding this comment.
Let's use the technically correct version "Unload and load"
|
I saw it, but don't have much capacity right now, sorry! We are preparing for the kilted release and have to triage things. |
|
@christophfroehlich no issues! best of luck for the release! we can take care of this later |
There was a problem hiding this comment.
Some RFCs..
offtopic: @soham2560 you can also help a lot here to do some reviews on open PRs :) And do you want to pickup #759 next? (starting from the stale existing PR)
| @@ -258,23 +258,20 @@ def _on_ctrl_menu(self, pos): | |||
| # TODO: use cleanup service once available | |||
There was a problem hiding this comment.
| # TODO: use cleanup service once available | |
| # TODO: use cleanup service once available | |
| # https://github.com/ros-controls/ros2_control/issues/759 |
| action_activate = menu.addAction(self._icons["active"], "Activate") | ||
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Cleanup") | ||
| action_activate = menu.addAction(self._icons["active"], "Activate (active)") | ||
| action_cleanup = menu.addAction(self._icons["unconfigured"], "Cleanup (unconfigured)") |
There was a problem hiding this comment.
Let's use the technically correct version "Unload and load"
| elif ctrl.state == "unconfigured": | ||
| action_configure = menu.addAction(self._icons["inactive"], "Configure") | ||
| action_spawn = menu.addAction(self._icons["active"], "Configure and Activate") | ||
| action_spawn = menu.addAction(self._icons["active"], "Configure and Activate (active)") |
There was a problem hiding this comment.
If we have everywhere only the direct options, why add this multi-hop here?
Brief
In this PR
rqt_controller_manager(refer 546c9c0)rqt_controller_manager#2151 (do refer the conversation at the issue for a more detailed discussion)How was this tested?
Current Logic
active🟢inactive🔵unconfigured🟡unloaded⚫