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
[pr2eus] Add key topic-name to :send-cmd-vel-raw #481
Conversation
Nice PR!! |
pr2eus/robot-interface.l
Outdated
(setq vel-topic | ||
(if topic-name | ||
(if namespace | ||
(if (eq (char "/" 0) (char topic-name 0)) topic-name (format nil "~A/~A" namespace topic-name)) | ||
topic-name) | ||
(if namespace (format nil "~A/~A" namespace cmd-vel-topic) cmd-vel-topic))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be better to use let
instead of setq
to set vel-topic
as a local variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I have revised it.
What do you mean? Why don't you just set the correct topic name in the :init function? Or do we have multiple base_controller servers? |
I just missed args in send-super*. I can set the topic in the :init function as you said. I would like to use :send-cmd-vel-raw with (pr2-init), but to make pr2-interface with topic name is substitutable for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another coding suggestion, in case you are fine on setting :namespace nil
for global topics (the first letter is not checked)
pr2eus/robot-interface.l
Outdated
(if namespace | ||
(if (eq (char "/" 0) (char topic-name 0)) topic-name (format nil "~A/~A" namespace topic-name)) | ||
topic-name) | ||
(if namespace (format nil "~A/~A" namespace cmd-vel-topic) cmd-vel-topic)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(:send-cmd-vel-raw
(x y d &key (topic-name cmd-vel-topic) ((:namespace ns) namespace)) ;; [m] [m] [degree]
(if ns (setq topic-name (format nil "~A/~A" ns topic-name)))
;; ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the helpful advice. It's simple and easy to understand.
Currently
:send-cmd-vel-raw
publishes topic set inrobot-move-base-interface :init
.This PR enables specifying topic name when calling the method. It is useful when you want to use subclass such as
pr2-interface
.Added key
topic-name
follows:send-cmd-vel-raw
in fetch-interface.