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

rename tactile touch to head touch #75

Merged

Conversation

kochigami
Copy link
Contributor

Based on the discussion in ros-naoqi/naoqi_bridge_msgs#4,
I renamed tactileTouch (for head touch sensor) to headTouch.
In addtion, put it and handTouch in an alphabetical order.

It requires ros-naoqi/naoqi_bridge_msgs#29 .

One concern is that even though renaming TactileTouch to HeadTouch would be more easy to follow for me, it may have an effect on TactileTouch topic users.

If there is any problem/ comment, please let me know.

Copy link
Member

@nlyubova nlyubova left a comment

Choose a reason for hiding this comment

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

Hi, could you please rename hand and head params in the config file to make them easier to understand?

@@ -78,11 +78,11 @@
{
"enabled" : true
},
"tactile":
Copy link
Member

Choose a reason for hiding this comment

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

can we name it tactile_hand or touch_hand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for your comment.
I renamed it to touch_hand.

{
"enabled" : true
},
"hand":
"head":
Copy link
Member

Choose a reason for hiding this comment

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

can we name it tactile_head or touch_head?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for your comment.
I also renamed it to touch_head.

Copy link
Member

@suryaambrose suryaambrose left a comment

Choose a reason for hiding this comment

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

lgtm, but I approve comments from @nlyubova

@kochigami
Copy link
Contributor Author

Thank you very much for your reviews.
I renamed boot_config parameter names of hand and head to touch_hand and touch_head.

@nlyubova
Copy link
Member

nlyubova commented Nov 7, 2016

great, thank you!

@nlyubova nlyubova merged commit a7c77cc into ros-naoqi:master Nov 7, 2016
@vrabaud
Copy link
Contributor

vrabaud commented Nov 7, 2016

@nlyubova, you need to release the new messages to take that into account, otherwise the buildfarm fails and you receive emails :)

@nlyubova
Copy link
Member

nlyubova commented Nov 7, 2016

@vrabaud right, I've also merged the corresponding PR in naoqi_bridge_msgs but then realized that there are many pending PR there and thus did not release it. No worries, will fix it!

@nlyubova
Copy link
Member

nlyubova commented Nov 8, 2016

@vrabaud @suryaambrose @Karsten1987 I've just released naoqi_bridge_msgs, sorry Karsten for releasing your package but otherwise naoqi_driver would complain ............. and I did not merge any other PR in naoqi_bridge_msgs; please decide if you prefer to merge (especially naoqi_apps related messages) or not

@Karsten1987
Copy link
Contributor

No problem. We can also transfer ownership as I have only little control over it from remote.

michieletto pushed a commit to michieletto/naoqi_driver that referenced this pull request May 24, 2017
…o-head-touch

rename tactile touch to head touch
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.

5 participants