-
Notifications
You must be signed in to change notification settings - Fork 1
Creates Arch.yml using rosnode list #80
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
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