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

Feature: Dynamically change polygons in collision monitor #3196

Closed
tonynajjar opened this issue Sep 15, 2022 · 11 comments
Closed

Feature: Dynamically change polygons in collision monitor #3196

tonynajjar opened this issue Sep 15, 2022 · 11 comments

Comments

@tonynajjar
Copy link
Contributor

tonynajjar commented Sep 15, 2022

Since the collision monitor is supposed to replace a safety scanner, I think it would make sense to emulate field set switching by subscribing to polygons.

Typically fieldsets would change according to speed/steering angle. one could write a separate node that subscribes to speed/steering angle and publishes which fieldset/polygon should be active.

Question here is: does it undermine the safety aspect of the collision monitor?

@SteveMacenski
Copy link
Member

SteveMacenski commented Sep 15, 2022

is supposed to replace a safety scanner

It is not supposed to replace a safety scanner, to be clear. It is a collision monitor which is not certified as a replacement for any safety equipment. I need to repeat that anytime something like that is said, using those particular words. I think you know what I mean here 😉

I'm not entirely clear what you're asking for. The polygons used are published that can be subscribed to. Are you asking for the ability to dynamically change the polygons?

@tonynajjar
Copy link
Contributor Author

tonynajjar commented Sep 15, 2022

Gotcha, wrong wording, I did see the discussion about that on Discourse.

Anyway, yes I mean to dynamically change polygons. Updated title

@tonynajjar tonynajjar changed the title Feature: Subscribe to polygons in collision monitor Feature: Dynamically change polygons in collision monitor Sep 15, 2022
@SteveMacenski
Copy link
Member

SteveMacenski commented Sep 15, 2022

Sure, yeah, I think that sounds reasonable. Your choices would be via dynamic parameters, topics, or services. What do you think there?

I might not tell people that's the recommended action, but certainly would be reasonable to have supported

Edit: or do you mean disabling/enabling several different pre-established polygons? Services there would make alot of sense.

@tonynajjar
Copy link
Contributor Author

tonynajjar commented Sep 15, 2022

Edit: or do you mean disabling/enabling several different pre-established polygons? Services there would make alot of sense.

Mmm, no I was actually thinking new unconfigured polygons for greater flexibility.

Your choices would be via dynamic parameters, topics, or services

Yeah good question. I would think topics because of the potentially high frequency of change (e.g if the switching is dependent on speed and steering angle). As far as I know topics would offer the least latency, but I might be wrong with that assumption.

@AlexeyMerzlyakov
Copy link
Collaborator

In Collision Monitor, we have already subscribing on footprints in case of APPROACH behavior model. So, I do not see any obstacle to have an ability to subscribe to any arbitrary polygon for the rest of models (STOP and SLOWDOWN).
In my opinion, topics is a good option for that. Maybe anyone have another thoughts, why the services in this case might be preferable over geometry_msgs/msg/Polygon (or PolygonStamped) messages?

@vinnnyr
Copy link
Contributor

vinnnyr commented Sep 16, 2022

+1 for topics. We too have a custom variant of this feature living elsewhere in the system, and like stated @tonynajjar stated here:

Typically fieldsets would change according to speed/steering angle.

we change the "zones" based on the current speed and current steering angle of our robot, maybe at a rate near 10hz. I don't know if it makes sense hitting a service at quite that rate.

@SteveMacenski
Copy link
Member

SteveMacenski commented Sep 16, 2022

Topics seems reasonable to me 👍

This should be a relatively straight forward contribution for someone interested 😄 Just need to expose a subscriber inside of the polygon class for changes in the stop/slow modes for overriding the polygon. Probably also a parameter for whether to use a parameter-defined static polygon or a dynamically changing one via a topic (but not both).

@AlexeyMerzlyakov
Copy link
Collaborator

Good. The major thing here was agreed:
The current functionality of polygons in the Collision Monitor to be enhanced by subscribing to the topics:
the similar way as APPROACH model utilizes. Current functionality with strictly-defined polygon will be intact.
This could be reached e.g. by adding a new polygon_topic parameter. If this parameter is not set, using points array with polygon vertexes. Otherwise - subscribing to the polygon topic.

I'll could add this functionality in next CM update.

@tonynajjar
Copy link
Contributor Author

Thank you very much!

@doisyg
Copy link
Contributor

doisyg commented Dec 5, 2022

Just to state that we just tested this new feature (mainly @kaichie) and that is exactly what we were looking for.

@mbed92
Copy link

mbed92 commented Jul 26, 2023

Does that functionality work on Humble? I need something like that to dynamically change polygons, but I cannot get it to work - params polygon_sub_topic (and polygon_topic) simply do not exist. I'm using the newest release, 1.1.8. However, polygon_sub_topic is mentioned even in polygon documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants