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

Docking backwards as plugin param #4695

Open
cgasconb opened this issue Sep 27, 2024 · 1 comment
Open

Docking backwards as plugin param #4695

cgasconb opened this issue Sep 27, 2024 · 1 comment

Comments

@cgasconb
Copy link

Feature request

The dock_backwards parameter as a dock_plugin parameter

Feature description

The dock_backwards param is a general param of the node, so it is fixed once the node starts. This way, the dock_plugins and dock instances are always forward or backward. In the case of needing a dock_plugin as forward and another one as backward, it is necessary to start 2 nodes changing this parameter.
As this param is called each time the velocity is calculated, it would be great if this param is part of the dock_plugin params, so we can have 2 plugins for forward and backward in one node.

@SteveMacenski
Copy link
Member

SteveMacenski commented Oct 1, 2024

This makes sense and you are not the first to bring this up, so I think this is worthwhile.

I took a look and it appears that most of the uses of dock_backwards_ are trivial to replace with a dock->plugin->dockDirection() or so forth. Then in the simple docking plugins, I can make the dock_direction a parameter so you can specify for each plugin instance which to use. Thus, you could have a forward and reverse plugin instance within a single server instance.

A couple of APIs would need to change (resetApproach to pass in the direction or the current plugin; move the parameter off the server) but nothing technically complicated.

I would propose the following:

  • The dock plugin base class has a bool dockForward() API. Optionally we could make the return type an enum with forward and reverse as current implementations, but leave room for more in the future (should there be lateral docking uses for omni robots)
  • At the start of dockRobot and undockRobot, obtain this in a local dock_direction parameter which can replace all uses of dock_backwards_. Pass this into resetApproach as well. This would be a bool of T/F, so the enum wouldn't be used. Maybe we could have the enum for lateral as another problem for another time.

I think that should work. Then the open question I have is:

  • How should we / should we support existing uses of dock_backwards_?

We could still get the parameter and log a warning that this is depreciated and to use the new dock plugin API. However in that case if we get it, we need to deconflict it when we use it in dockRobot and undockRobot.

Alternatively, we could have the simple plugins have their own <dock name>.dock_direction parameter, and if that's not set, look for the global dock_backwards parameter instead to use. Then also log that its depreciated and use <dock name>.dock_direction instead. The deconfliction is that if its set on the plugin-level, the global-level deprecated parameter is never obtained.

I think the latter is cleaner, but works primarily for users of the simple charging / non-charging plugins provided. I suppose that is 'fine' as long as we don't backport these changes and only available in Rolling and newer. I'm not sure we could backport the changes anyway since we change the API of resetApproach.

What do you think? Interest in contributing this or something I should work on on the way to ROSCon?

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

No branches or pull requests

2 participants