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

Handles empty globes properly #297

Merged
merged 9 commits into from
Oct 2, 2017
Merged

Conversation

Behery
Copy link
Member

@Behery Behery commented Sep 28, 2017

The api is now able to handle empty globs. Meaning that the service calls are now functional even if all the topics/services/params are exposed. Fixes #296

@Behery Behery changed the title Handles empty globes properly, Fixes #296 Handles empty globes properly Sep 28, 2017
@jihoonl jihoonl self-requested a review September 28, 2017 16:12
Copy link
Member

@jihoonl jihoonl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Only small nits.

if(possible_action_server == namespace and topic == "result"):
possibility[3] = 1
if(possible_action_server == namespace and topic == "status"):
possibility[4] = 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this method too.
filter_action_servers method can be simplified like this way. Could you make this change?

    possibility = [0, 0, 0, 0, 0]
    action_topics = ['cancel', 'feedback', 'goal', 'result', 'status']
    for topic in sorted(topics):
        if (len(topic.split('/')) == 3):
            [empty, namespace, topic] = topic.split('/')
            if(possible_action_server != namespace):
                possible_action_server = namespace
                possibility = [0, 0, 0, 0, 0]
            if possible_action_server == namespace and topic in action_topics:
                possibility[action_topics.index(topic)] = 1
        if all(p == 1 for p in possibility):
            action_servers.append(possible_action_server)

@@ -45,16 +45,20 @@

from rosapi.msg import TypeDef

from glob_helper import filter_globs, any_match
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use from .glob_helper import filter_globs, any_match to import from local module.

@jihoonl jihoonl self-assigned this Sep 29, 2017
@Behery
Copy link
Member Author

Behery commented Oct 2, 2017

Hi @jihoonl could you please check the latest updates?
Thanks

@jihoonl
Copy link
Member

jihoonl commented Oct 2, 2017

Thanks.

@jihoonl jihoonl merged commit 76a9bc4 into RobotWebTools:develop Oct 2, 2017
jihoonl pushed a commit that referenced this pull request Oct 16, 2017
* Refactors get_globs function to a separate module
* Refactors the filtering that uses the globs
* Some linting
* Handles topic types for empty globs
* Refactors out an any_match function
* Simplifies filter_action_servers
* Imports socket for the errors
* Uses import .glob_helper
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.

None yet

2 participants