Open 708yamaguchi opened 3 years ago
It would also be nice to have some more debate and some other points of view on the changes below:
Removing the :unsubscribe
argument.
I am kind of comfortable with this change since I couldn't find any occurrences in the jsk_robot and pr2eus, but I'm not sure about hrpsys and tendon robots. If backwards compability is a must I guess we could also keep the key arg and just print a "deprecated" warning and unsubscribe it anyway?
How to reject calls if prior subscribers are found.
Most of the roseus functions log errors and return nil, but I think in this case it might be good to raise standard euslisp errors as well, since the program is most likely to briefly fail with something like cannot find method :... in ...
when unsuccessful.
In some other minor comments:
ros::get-topic-subscriber
instead of ros::get-num-publishers
? This seems a lot more intuitive to me. Thank you very much for your review. I updated the code and pushed the commits.
I too would like to ask other people's ideas.
@tkmtnt7000
For detail, please see https://github.com/jsk-ros-pkg/jsk_roseus/issues/666
There are several cases where (one-shot-subscribe) occurs an error.
In this PR,
:unsubscribe
argument and checking subsciber. This makes sure that there is no subscriber before (while ... (ros::spin-once)) is called. (the first commit of this PR)My concern is that by removing the :unsubscribe argument, backward compatibility will be lost.
Thank you very much for your help. @YoheiKakiuchi @pazeshun @Affonso-Gui