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

Dock instance level parameters and dock_backwards depending on dock plugin #44

Closed
redvinaa opened this issue Jun 26, 2024 · 6 comments
Closed

Comments

@redvinaa
Copy link
Contributor

I have a few questions about parameter scopes, as the title suggests.

Firstly, how do you deal with docking stations that are close to each other, each of the same type but different markers. As I understand, the instantiated dock plugins cannot have arbitrary parameters, so these would have to be implemented as different dock types and set the marker id parameter on the plugin level, right?

The other question is, doesn't it make sense to set the dock_backwards parameter on a per dock type basis? For example on a delivery robot, where the trays face to the front, it would dock to tables forward. And if the charging outlet is on the back, it would dock backwards.

@SteveMacenski
Copy link
Member

As I understand, the instantiated dock plugins cannot have arbitrary parameters

Why not? The dock plugins could themselves have parameters that you could create and read in. But, if there's just N instances, that's not really a huge worry since you send it to one in particular at a known location so it should stage w.r.t. that dock, not the others. Then, I suppose its a question about how you detect your intended dock versus a neighboring dock if multiple existing your camera frame. You could do a number of things that are pretty straight forward, I'll just enumerate a couple off the cuff thoughts:

  • If you have a detector node based on markers, have each dock have a unique marker so that you can tell which one in frame is precisely the one you're interested in
  • Set the camera / detector node as cropped to only consider what's in front of it +/- some margin
  • If you have a detector node that detects all the docks, use the one closest to the center of the frame
  • etc

It highly depends on your situation, detection type, and how close they really are practically. This has been tested in a room with a wall full of docks without a problem.

doesn't it make sense to set the dock_backwards parameter on a per dock type basis?

Doesn't that example make the dock backwards a feature of the robot platform? The robot's charging contacts are in the front/back no matter what specific dock is used.

@redvinaa
Copy link
Contributor Author

If you have a detector node based on markers, have each dock have a unique marker so that you can tell which one in frame is precisely the one you're interested in

This is what I would like to do, but as far as I can see, parameters can only be set at the dock type (plugin) level, not the specific dock with specific marker id (instance) level. (Aren't I right? The instance parameters are hardcoded here.

Or maybe you're referring to creating a different type for each marker and have just one instance of each? Though I thought the goal was to have few types and many instances.


Doesn't that example make the dock backwards a feature of the robot platform? The robot's charging contacts are in the front/back no matter what specific dock is used.

Well no, if we take "docking" in the broader sense of not only docking to the charger but also to tables, shelves etc.

@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 27, 2024

parameters can only be set at the dock type (plugin) level, not the specific dock with specific marker id (instance) level

Interesting, yeah that would be the more natural place, wouldn't it? I think we could add in an optional id string field - I assumed the name would be dual purpose to embed some information about the dock's ID but I think this is a good point that the semantic name of the dock might not be sufficient. What do you think? We could try to generalize this further, but I'd rather not over-complicate things unless we know that someone needs more than just an additional std::string id. I'd just add the id and if someone come with even more, then make it more generalized.

I was thinking that the dock plugin itself could have the list of dock tag IDs within it, but that's alot less natural of a solution than having the dock instances themselves containing their attributes.

Well no, if we take "docking" in the broader sense of not only docking to the charger but also to tables, shelves etc.

I suppose it didn't occur to me that someone would use a single docking server instance for both charge-docking and infra-docking. I would have made 2 to keep things separated, since they're very lightweight and have a /dock_robot_at_station and /charge_robot action. Though, we don't currently support non-charging dock things, but that's covered in this ticket ros-navigation/navigation2#4405 and might be a good point to make over there to shift that around when working on that. We can make that adjustment when non-charging docking support is introduced.

@redvinaa
Copy link
Contributor Author

I assumed the name would be dual purpose

Honestly this didn't occur to me, it could work just fine. Then I'm not even sure we need the additional id field. Do you think it's needed? Otherwise we can close this.

I would have made 2 to keep things separated, since they're very lightweight

Also didn't think of it, then I guess it is solved.

@SteveMacenski
Copy link
Member

Then I'm not even sure we need the additional id field.

Honestly I think its a good idea. That way you can have kitchen_dock and have an id that is the apriltag ID or other identifier that can disambiguate. If you open a PR on that, happy to merge and should be < 20 lines with tests.

Also didn't think of it, then I guess it is solved.

Still worth filing on the ticket I linked to since I think this is a good detail I missed in the original design.

@redvinaa redvinaa mentioned this issue Jul 1, 2024
@redvinaa
Copy link
Contributor Author

redvinaa commented Jul 1, 2024

I opened a PR (#46) to add the id.
The rest of the conversation can go on there, and in the mentioned ticket about the other question. I'm closing this one.

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