Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Created public get_topic_list() function for use in other scripts.#1146

Closed
EliteMasterEric wants to merge 12 commits intoros:lunar-develfrom
EliteMasterEric:topic-list
Closed

Created public get_topic_list() function for use in other scripts.#1146
EliteMasterEric wants to merge 12 commits intoros:lunar-develfrom
EliteMasterEric:topic-list

Conversation

@EliteMasterEric
Copy link
Copy Markdown
Contributor

@EliteMasterEric EliteMasterEric commented Aug 18, 2017

Resolves issue #946.

The only function in rostopic for listing the available topics is built exclusively for the command line interface; this adds a public function for other applications that want a list of topics.

Usage:

import rostopic
publishers, subscribers = rostopic.get_topic_list()
for topic, type, node_count in publishers:
    print("Publisher: %s, [%s] (%d nodes)" % (topic, type, node_count))
for topic, type, node_count in subscribers:
    print("Subscriber: %s, [%s] (%d nodes)" % (topic, type, node_count))

@dirk-thomas
Copy link
Copy Markdown
Member

After extracting the print-agnostic part of the existing functionality as you did in the current patch please also update the existing code to use this new function (rather than duplicating this logic). Please also consider adding a test for the new function.

@EliteMasterEric
Copy link
Copy Markdown
Contributor Author

@dirk-thomas Changes have been made, removing redundancy and adding a test to the rostopic test script.

Copy link
Copy Markdown
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

Please make sure to run all code parts you have changed in all possible variations to make sure the patch actually works (which is currently not the case).

topic_types = _master_get_topic_types(master)

pubs, subs, _ = state
publishers = []
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line is overwriting the function parameter.

Same below for subscribers.

publishers = []
if publishers:
for topic, nodes in pubs:
publishers.append(topic, topic_type(topic, topic_types), len(nodes))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

list.append() only accepts a single argument.

Same below.

topic_ns = rosgraph.names.make_global_ns(topic)
subs = (x for x in subs if x[0] == topic or x[0].startswith(topic_ns))
pubs = (x for x in pubs if x[0] == topic or x[0].startswith(topic_ns))
pubs, subs = get_topic_list()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new function returns different information than getSystemState before. I doubt that all following code continues to work as is.

The existing rostopic_list functions output straight to print() calls, so they are only functional for the CLI. Resolves issue ros#946.
Added new public function to tests.

Updated functions to use new public function to remove redundancy.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants