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

Fix TFClient by removing, rather than adding, a leading '/' character in TFClient.processFeedback() #79

Merged
merged 1 commit into from
May 22, 2014

Conversation

T045T
Copy link
Contributor

@T045T T045T commented May 16, 2014

Fixes issue #78

This is in accordance with both the hydro migration guidelines and the implementation of TFClient.subscribe() (TFClient.js:102)

This is in accordance with both the hydro migration guidelines [1] and the implementation of TFClient.subscribe() (TFClient.js:102)

[1]: http://wiki.ros.org/hydro/Migration#tf2.2BAC8-Migration.Removal_of_support_for_tf_prefix
@rctoris
Copy link
Contributor

rctoris commented May 17, 2014

I will test this out next week on Groovy-Indigo to make sure we have cross-version support.

@rctoris rctoris self-assigned this May 17, 2014
@T045T
Copy link
Contributor Author

T045T commented May 21, 2014

Actually, it's not only in the hydro guidelines, but the tf2 migration page as well - tf2 removes all leading slashes anyway, and since TfClient connects to tf2_web_republisher, removing any leftover slashes (that may be present in, f.i., the TfClient.fixedFrame parameter) should be fine :)

Also, I apologize - the original comment was misleading: It's not all that important whether the slash is added or removed, but it was beind added in one place and removed in another (TfClient.js:102), which left the TfClient unable to find the values in its dict.

rctoris added a commit that referenced this pull request May 22, 2014
Fix TFClient by removing, rather than adding, a leading '/' character in TFClient.processFeedback()
@rctoris rctoris merged commit 39c1b56 into RobotWebTools:devel May 22, 2014
k-aguete pushed a commit to k-aguete/roslibjs that referenced this pull request Oct 21, 2022
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