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

Feature/head and sensorring #593

Merged

Conversation

floweisshardt
Copy link
Member

@floweisshardt floweisshardt commented Jan 19, 2017

  • integrates head and sensorring on one bus
  • adapts diagnostics

extends #568

@floweisshardt
Copy link
Member Author

tested on cob4-10: OK

Copy link
Member

@fmessmer fmessmer left a comment

Choose a reason for hiding this comment

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

minor/questions

@@ -116,21 +116,22 @@
<arg name="robot" value="$(arg robot)"/>
<arg name="component_name" value="torso"/>
<arg name="can_device" value="can0"/>
<arg name="cartesian_control" value="true"/>
Copy link
Member

Choose a reason for hiding this comment

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

could you not just set the value to false?

here and wherever it applies...

Copy link
Contributor

Choose a reason for hiding this comment

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

No, the default be kept in a single place.

# sensorring:
# type: diagnostic_aggregator/GenericAnalyzer
# path: Sensorring
# contains: 'sensorring/driver'
# contains: 'sensorring/'
Copy link
Member

Choose a reason for hiding this comment

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

why keep it commented?

Copy link
Member Author

Choose a reason for hiding this comment

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

cob4-1 doesn't (yet) have a working sensorring hardware

type: diagnostic_aggregator/GenericAnalyzer
path: twist_mux
contains: 'twist_mux'
contains: 'base/'
Copy link
Member

Choose a reason for hiding this comment

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

what happened to the twist_mux diagnostics?
doesn't this contains pattern conflict with e.g. base/light?

Copy link
Member Author

Choose a reason for hiding this comment

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

light on base is using light_base namespace, so filtering works as expected

@fmessmer
Copy link
Member

I put a note about the high load problem observed on real hardware caused by the cartesian_control here...will work on this as soon as I find the time

@floweisshardt floweisshardt merged commit 47f837e into ipa320:indigo_dev Jan 20, 2017
@floweisshardt floweisshardt deleted the feature/head_and_sensorring branch July 19, 2017 11:46
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.

3 participants