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

[jsk_tools] Add delay_timestamp.py #1216

Merged
merged 2 commits into from
Nov 12, 2015

Conversation

wkentaro
Copy link
Member

@wkentaro wkentaro commented Nov 6, 2015

@wkentaro
Copy link
Member Author

Please merge.

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

you do not have to pass msg type, see http://wiki.ros.org/topic_tools

@wkentaro
Copy link
Member Author

I updated the commit.

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

by the way , when you use this tool? I though we do not need this is you crony all computers and use message filter.

@wkentaro
Copy link
Member Author

This is not for demo, but test for nodes which uses message_filters.
like: jsk-ros-pkg/jsk_recognition#1288

@wkentaro
Copy link
Member Author

@k-okada
PS. 英語のtypoをもう少し減らしていただけると読みやすくて助かります。。

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

add ros::warn to describe this is just for test code

◉ Kei Okada

On Tue, Nov 10, 2015 at 9:24 PM, Kentaro Wada notifications@github.com
wrote:

This is not for demo, but test for nodes which uses message_filters.
like: jsk-ros-pkg/jsk_recognition#1288
jsk-ros-pkg/jsk_recognition#1288


Reply to this email directly or view it on GitHub
#1216 (comment)
.

@wkentaro
Copy link
Member Author

instead of that, why don't we add doc for it?

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

doc is ok, but people who use this without knowing this is designed for the
system never reads the doc.

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:25 PM, Kentaro Wada notifications@github.com
wrote:

instead of that, why don't we add doc for it?


Reply to this email directly or view it on GitHub
#1216 (comment)
.

@wkentaro
Copy link
Member Author

what do you mean with 'the system'?

2015年11月10日火曜日、Kei Okadanotifications@github.comさんは書きました:

doc is ok, but people who use this without knowing this is designed for the
system never reads the doc.

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:25 PM, Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

instead of that, why don't we add doc for it?


Reply to this email directly or view it on GitHub
<
#1216 (comment)

.


Reply to this email directly or view it on GitHub
#1216 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

@wkentaro
Copy link
Member Author

and why do they use this node without reading doc or code?
(so should I comment about this in the code?)

if you worry about the limited use case, we can create a new package
'jsk_test_tools' as jsk_topic_tools.

2015年11月10日火曜日、Kentaro Wadawww.kentaro.wada@gmail.comさんは書きました:

what do you mean with 'the system'?

2015年11月10日火曜日、Kei Okada<notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>さんは書きました:

doc is ok, but people who use this without knowing this is designed for
the
system never reads the doc.

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:25 PM, Kentaro Wada notifications@github.com
wrote:

instead of that, why don't we add doc for it?


Reply to this email directly or view it on GitHub
<
#1216 (comment)

.


Reply to this email directly or view it on GitHub
#1216 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

和田 健太郎 / Kentaro Wada
http://wkentaro.com

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

system means everything but test code

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:36 PM, Kentaro Wada notifications@github.com
wrote:

what do you mean with 'the system'?

2015年11月10日火曜日、Kei Okadanotifications@github.comさんは書きました:

doc is ok, but people who use this without knowing this is designed for
the
system never reads the doc.

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:25 PM, Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

instead of that, why don't we add doc for it?


Reply to this email directly or view it on GitHub
<

#1216 (comment)

.


Reply to this email directly or view it on GitHub
<
#1216 (comment)

.

和田 健太郎 / Kentaro Wada
http://wkentaro.com


Reply to this email directly or view it on GitHub
#1216 (comment)
.

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

On Tue, Nov 10, 2015 at 10:40 PM, Kentaro Wada notifications@github.com
wrote:

and why they use this node without reading doc or code?

because the node exists.

◉ Kei Okada

@wkentaro
Copy link
Member Author

because the node exists.

because the node is necessary for their system, isn't it? If so, they should read the documentation (or code).

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

they somehow find the node and misinterpret the usage, I'm pretty sure that
someone will use this node to avoid timestamp problem in next 6 month,
believe me.

◉ Kei Okada

On Tue, Nov 10, 2015 at 10:52 PM, Kentaro Wada notifications@github.com
wrote:

because the node exists.

because the node is necessary for their system, isn't it? If so, they
should read the documentation (or code).


Reply to this email directly or view it on GitHub
#1216 (comment)
.

@wkentaro
Copy link
Member Author

So you mean they misunderstand this as substitute for topic_tools/throttle.

I think I understand your concern, and I'd like to avoid that possibility by putting it another package. could I create a new package? (jsk_test_tools) to serve nodes for test like
jsk_tools/test_stdout.py.

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

people who do not read document will think this is a magic node that solves
all tf problem, so crating new package did not solve, just print out BIG
warning message.

◉ Kei Okada

On Tue, Nov 10, 2015 at 11:03 PM, Kentaro Wada notifications@github.com
wrote:

So you mean they misunderstand this as substitute for topic_tools/throttle.

I think I understand your concern, and I'd like to avoid that possibility
by putting it another package. could I create a new package?
(jsk_test_tools) to serve nodes for test like
jsk_tools/test_stdout.py.


Reply to this email directly or view it on GitHub
#1216 (comment)
.

@wkentaro
Copy link
Member Author

Okay.
I believe your idea that comes from experience,
and will add warning message.

Maybe that kind of people overlook the warning message, so you say BIG ;)

2015年11月10日火曜日、Kei Okadanotifications@github.comさんは書きました:

people who do not read document will think this is a magic node that solves
all tf problem, so crating new package did not solve, just print out BIG
warning message.

◉ Kei Okada

On Tue, Nov 10, 2015 at 11:03 PM, Kentaro Wada <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');>
wrote:

So you mean they misunderstand this as substitute for
topic_tools/throttle.

I think I understand your concern, and I'd like to avoid that possibility
by putting it another package. could I create a new package?
(jsk_test_tools) to serve nodes for test like
jsk_tools/test_stdout.py.


Reply to this email directly or view it on GitHub
<
#1216 (comment)

.


Reply to this email directly or view it on GitHub
#1216 (comment)
.

和田 健太郎 / Kentaro Wada
http://wkentaro.com

@wkentaro
Copy link
Member Author

I added the warning message.
image

@k-okada
Copy link
Member

k-okada commented Nov 10, 2015

thank you

@wkentaro
Copy link
Member Author

Please merge.

k-okada added a commit that referenced this pull request Nov 12, 2015
[jsk_tools] Add delay_timestamp.py
@k-okada k-okada merged commit b4bd7d2 into jsk-ros-pkg:master Nov 12, 2015
@wkentaro wkentaro deleted the delay-timestamp branch November 12, 2015 10:11
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.

2 participants