search

fkie / multimaster_fkie

ROS stack with FKIE packages for multi-robot (discovering, synchronizing and management GUI)
BSD 3-Clause "New" or "Revised" License
271 stars 108 forks source link

Increased logging in master sync. #52

Closed deng02 closed 7 years ago

deng02 commented 7 years ago

Added more logging around synchronization to help with tracking changes in the local ROS master due to multimaster. Also added which masters are returned by master discovery and split out the horrible hack message to help with readability (was very dense for when there are alot of publishers in the list).

atiderko commented 7 years ago

Hi deng02,

thank you for improving and this pull request. Unfortunately, this leads to this error (to unsubscribe we need no topictype): [ERROR] [WallTime: 1478760398.801523] SyncThread[tiderko_11312] ERROR: Traceback (most recent call last): File "/ros/src/multimaster_fkie/master_sync_fkie/src/master_sync_fkie/sync_thread.py", line 308, in _apply_remote_state self.name, node, nodeuri, topic, topictype) UnboundLocalError: local variable 'topictype' referenced before assignment I think, it will be the same at line 339.

Can you fix this, please?

deng02 commented 7 years ago

Updated PR with the variable removed. Thanks for the catch!

deng02 commented 7 years ago

Hi atiderko,

I was wondering if it was possible to have this merged in? Or are there any other concerns? I'd be happy to address any issues.

Denise

atiderko commented 7 years ago

Hi deng02,

thank you for contributing!

It was possible to merge into the branch because there are no (unit/ros)tests defined to find a runtime error. Someone should create one ;-). So github checks only for conflicts in the code.

Alex

deng02 commented 7 years ago

Thanks so much for the merge! The failure was totally my fault, I should have written a unit test to more properly test my change :) and submitted it. I will do that next itme.