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.

Sign up for GitHub

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

Jump to bottom

[jsk_tools] Add node to publish diagnostics based on topic and node status #4

Merged
merged 15 commits into from
May 23, 2022
Merged

[jsk_tools] Add node to publish diagnostics based on topic and node status #4

merged 15 commits into from
May 23, 2022

Conversation

708yamaguchi
Copy link
Collaborator

Duplicate of jsk-ros-pkg#1727

I want robots to talk about the status of diagnostics using jsk-ros-pkg#1607.

@708yamaguchi 708yamaguchi changed the title [jsk_tools] Return value of checkNodeState [jsk_tools] Add node to publish diagnostics based on topic and node status May 12, 2022
This was referenced May 12, 2022
Merged
Closed
@708yamaguchi
Copy link
Collaborator Author

@iory

Could you review this?

@708yamaguchi
Copy link
Collaborator Author

@knorth55

Could you merge this or give me merge permission?
jsk-ros-pkg PR is merged.

708yamaguchi added 14 commits May 23, 2022 07:59
@708yamaguchi
[jsk_tools] Return value of checkNodeState
88e772b
@708yamaguchi
[jsk_tools] Add node to publish /diagnostics based on topic and node …
c8f3912 
…status
@708yamaguchi
[jsk_tools] Add diagnostic dependency
7afd0f8
@708yamaguchi
[jsk_tools] Add documentation for sanity_diagnostics
e26681a
@708yamaguchi
[jsk_tools] Add sample for sanity_diagnostics
9b4a5bf
@708yamaguchi
[jsk_tools] Add test for sanity_diagnostics
40ed7cb
@708yamaguchi
[jsk_tools] Fix typo
90c563e
@708yamaguchi
[jsk_tools] Use topic name as default value
bf0a63d
@708yamaguchi
[jsk_tools] Do not check topic type until the first topic comes in
ccc9e95
@708yamaguchi
[jsk_tools] Unregister subscriber at the end of checkTopicIsPublished…
ee2d0ba 
… to reduce CPU load
@708yamaguchi
[jsk_tools] Set longer timeout for low frequency topics
f41f161
@708yamaguchi
[jsk_tools] Install sample directory
38294f5
@708yamaguchi
[jsk_tools] Fix test name for sanity_diagnostics
dc45f3b
@708yamaguchi
[jsk_tools] Set longer timeout for sanity_diagnostics test
3edfa96
@708yamaguchi 708yamaguchi force-pushed the sanity-diagnostics-fetch15 branch from 0af6e3a to 3edfa96 Compare May 22, 2022 22:59
@708yamaguchi
Copy link
Collaborator Author

@iory

#6 のsample_audible_warning.launchを実行すると、エラーとstaleが出ます

エラーの原因は確認できていないのですが、少なくともstaleの方は、このプルリクエストで使っているdiagnostics_analyzer.yamlが#6 でも使われているからだと思います。

矢野倉さん的には、同じdiagnostics_analyzer.yamlを使いまわすのがよいと考えていますか？

audible

@708yamaguchi
Copy link
Collaborator Author

あと個人的TODOですが、PR1040体内のjsk_toolsがpr1040-with-audible-warningブランチのままになっているので、このプルリクエストに相当するプルリクエストをpr1040ブランチ？に出さないといけないです

@iory
Copy link
Collaborator

iory commented May 23, 2022

矢野倉さん的には、同じdiagnostics_analyzer.yamlを使いまわすのがよいと考えていますか？

これは意図的に同じにしていたのですが、staleが出ていると紛らわしいということなので、分割しました。
jsk-ros-pkg@7b4e673

@708yamaguchi
Copy link
Collaborator Author

708yamaguchi commented May 23, 2022 •
edited
Loading

ご対応ありがとうございました。
cherry-pickしました。

@knorth55
fetch実機でも試していて問題なさそうなのですが、マージしていただけますか？
あと、PR2用ブランチにもプルリクエストを送りたいのですが、pr1040ブランチってありますか？

@knorth55 knorth55 merged commit 4651c4a into knorth55:fetch15 May 23, 2022
@knorth55 knorth55 mentioned this pull request May 23, 2022
Merged
@knorth55
Copy link
Owner

@708yamaguchi #7 これでいいかな

@708yamaguchi
Copy link
Collaborator Author

@knorth55

わざわざすみません，ありがとうございます．
助かりました．

@708yamaguchi 708yamaguchi mentioned this pull request May 24, 2022
Merged
@708yamaguchi 708yamaguchi mentioned this pull request May 25, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@708yamaguchi @iory @knorth55