-
Notifications
You must be signed in to change notification settings - Fork 1
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
Creates Arch.yml using rosnode list #80
base: master
Are you sure you want to change the base?
Conversation
Changes to be committed: new file: src/rosdiscover/Issue74.py
src/rosdiscover/Issue74.py
Outdated
image = 'therobotcooperative/turtlebot3' | ||
sources = ['/opt/ros/kinetic/setup.bash', '/ros_ws/devel/setup.bash'] | ||
environment = {'TURTLEBOT3_MODEL': 'burger'} | ||
with rsw.launch(image, sources, environment=environment) as system: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's refactor this into a function
src/rosdiscover/Issue74.py
Outdated
service_to_format[service_name] = service.format.name | ||
|
||
|
||
nodeSummaryDict = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would refactor this a separate function too. That takes a state and returns NodeSummaryDict.
I think also, for this, you could try using mypy typing like we do in the other parts of the code.
modified: src/rosdiscover/cli.py
I know that the checks will fail... I just needed to push whatever work I had so that I could pull it from another computer. |
That's perfectly fine! 🙂 You should feel free to make more, smaller commits, rather than worrying about making fewer, larger ones. Also, don't forget to add short but descriptive messages to your commits. |
deleted: src/rosdiscover/Issue74.py modified: src/rosdiscover/cli.py modified: src/rosdiscover/models/gazebo_gui.py modified: src/rosdiscover/models/joy.py modified: src/rosdiscover/models/twist_mux.py Added 'dynamic' feature to cli.py that dynamically analyzes nodes.
src/rosdiscover/cli.py
Outdated
package=package, | ||
args={'gui': 'false'}) | ||
|
||
time.sleep(30) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this as an optional parameter, but default it to 30.
src/rosdiscover/cli.py
Outdated
environment = data['environment'] | ||
node_names, state, topic_to_type, service_to_format = get_info(image, sources, environment, | ||
args.launchfile, args.package) | ||
nodeSummaryDict = create_dict(node_names, state, topic_to_type, service_to_format) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the following code can be done more simply, like we discussed yesterday
node_names, state, topic_to_type, service_to_format = get_info(image, sources, environment, | ||
args.launchfile, args.package, sleep_time) | ||
node_summary_dict = create_dict(node_names, state, topic_to_type, service_to_format) | ||
f.write(json.dumps(node_summary_dict, indent=4, separators=(". ", " = "))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separators
argument here should be dropped to avoid producing bad JSON.
Changes to be committed:
new file: src/rosdiscover/Issue74.py