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

Add websocket header support to the ROS-client #124

Merged
merged 5 commits into from
Oct 25, 2024

Conversation

TheSpeedM
Copy link
Contributor

@TheSpeedM TheSpeedM commented Jun 17, 2024

Description

Some device that I am using has a websocket available that is protected with a token. This token needs to be passed via the websocket connection header. This PR adds websocket header support to the ROS-client.

The headers can be added to the ROS-client like this, also see test_ws_headers.py:

headers = {
    'cookie': 'token=rosbridge',
    'authorization': 'Some auth'
}

client = Ros('127.0.0.1', 9000, headers=headers)

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

I don't see a reason why this couldn't be backwards compatible, but I could be wrong.

Checklist

  • I added a line to the CHANGELOG.rst file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke check).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

Questions

  • Is this desired for this library? Otherwise I will continue to use my fork.
  • Is the test in the right spot? It is independent of ROS-version so this seemed fine.
  • I am not familiar with autobahn and twisted so let me know if this should be accomplished in another way.
  • Should the passing of headers be more explicit? Now it just happens via the **kwargs of RosBridgeClientFactory.

@gonzalocasas
Copy link
Member

Cool! Thanks!

@TheSpeedM
Copy link
Contributor Author

Hi @gonzalocasas, any update on getting this merged?

@Elian0213
Copy link

Hi @gonzalocasas is this going to get merged? I had to pull his repository seperately in order to get this functionality working.

Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it took me so long! it lgtm! I'll merge and release today

@gonzalocasas gonzalocasas merged commit d36dd0e into gramaziokohler:main Oct 25, 2024
6 of 7 checks passed
@Elian0213
Copy link

Sorry it took me so long! it lgtm! I'll merge and release today

Hell yeah!

@gonzalocasas
Copy link
Member

gonzalocasas commented Oct 25, 2024

@TheSpeedM @Elian0213 the release is already available on pip, will land on conda-forge a bit later today/early tomorrow

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.

3 participants