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

Restrictions reworked for ROS2 utilising the eval system to enable more modularity at runtime #168

Draft
wants to merge 5 commits into
base: humble-dev
Choose a base branch
from

Conversation

Iranaphor
Copy link
Contributor

This is a draft for an alternative restrictions handling system which utilises the python eval functionality in an efficient manner to filter out nodes which do not match a given condition.

Some examples are included towards the end of restrictions_handler.py showing how this can be utilised with the existing restriction conditions in practice, while also offering new facilities.

A key limitation with the old approach was the assumption that planning restrictions were only capable of being categorised as robot type or task type, and new code needs to be added for any other categorising. This alternative approach by contrast is much more dynamic.

Additionally a qol update offers the inclusion of the restriction condition in the launch file so this is not needed to be published from a separate node (though this can still be done to offer dynamic changes).

I still have to include a ROS2 launch file and fully test the functionality, but am curious in the meantime to hear the thoughts of @marc-hanheide and @francescodelduchetto as to whether this is a reasoned approach.

@Iranaphor Iranaphor self-assigned this Nov 23, 2023
@Iranaphor Iranaphor marked this pull request as ready for review November 23, 2023 10:36
@marc-hanheide marc-hanheide marked this pull request as draft December 4, 2023 11:55
@marc-hanheide
Copy link
Member

@Iranaphor where are we with this?

@Iranaphor
Copy link
Contributor Author

It is functional and working. I will add some example maps, test cases and documentation. I will try to merge in mid Feb if there are no problems.

@marc-hanheide
Copy link
Member

@Iranaphor this has been rather stale as a draft… Where are we with this? Copying in @ibrahimhroob to consider how we best move forward as we have the ambition to release a new toponav soon-ish.

@Iranaphor
Copy link
Contributor Author

Iranaphor commented Nov 8, 2024

As this could potentially break existing restrictions functionality in preference to the new dynamic approach, it needs some caution.

Three examples are included at: https://github.com/Iranaphor/topological_navigation/blob/a7125b3d64bb02497753d90440c0c9427e31e63c/topological_navigation/topological_navigation/scripts/restrictions_handler.py#L146

# Example 1: passed as list
node_restriction = "['short', 'tall']"
msg_data = "'short' in $"

# Example 2: passed as string
node_restriction = "robot_short & robot_tall"
msg_data = "'robot_short' in '$'"

# Example 3: passed bool
node_restriction = "'True'"
msg_data = "'robot_short' in '$' or '$' == 'True'"

The node_restriction is what is contained within the tmap file, msg_data is what is either passed through a topic, or loaded from the launch file. The system evaluates each edge/node using the logic:

eval(msg_data.replace('$', node_restriction))

In the tests I have conducted it seems to work, however I am cautious to say there wont be scenarios which break it.

Due to this, it may be better to include the new restrictions under a new tag in the map file, though if it is to work as a flat replacement for the prior system (as intended) then this would be unnecessary. This may lead in further to the necessity of including tmap versioning.

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

Successfully merging this pull request may close these issues.

2 participants