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

SlimServer does not work with the off-the-shelf cometd library from npm - I think it can with very small changes. #765

Closed
lynniemagoo opened this issue Feb 20, 2022 · 3 comments
Assignees
Labels

Comments

@lynniemagoo
Copy link
Contributor

lynniemagoo commented Feb 20, 2022

I have done quite a bit of research and testing with the cometd library available from npm. I first started working with 5.0.0 version of the library and got things working using the long-polling version of the cometd client subscriptions. The client library does not support streaming/pipeline subscriptions but I don't need this for my application just yet.

In order to get a client working that properly handles callbacks, the cometd client uses an 'id' field in each message to the server and upon receiving a response, uses the echoed 'id' to notifiy callbacks and listeners.
The library properly sends the messages to slimserver but upon responding, the slimserver event does not include the incoming 'id' field. From my initial observation of the comet implementation in slimserver, I see that the code extracts the clientId from each message and echoes that in the corresponding event. I'm wondering if a fix would be as simple as including the original 'id' field if it exists in each emitted event.

@mherger - I'm prepared to do a fork and a PR but don't have the ability to build slimserver locally.

Comments are appreciated.

@lynniemagoo
Copy link
Contributor Author

lynniemagoo commented Feb 20, 2022

@mherger - I just confirmed the local changes I made to 5.0.0 version of cometd to allow to work with LMS also apply to the latest 7.0.5 version from npm. I created 2 PRs for this issue for both v8.2 and v8.3.

@mherger
Copy link
Contributor

mherger commented Feb 20, 2022

Fixed via #767.

@mherger mherger closed this as completed Feb 20, 2022
@lynniemagoo
Copy link
Contributor Author

@mherger Fix confirmed and tested using 8.3 nightly for Windows.

http://downloads.slimdevices.com/nightly/8.3/lms/99bdfa09c450745600b4513ae5ec9fe7e9026087/LogitechMediaServer-8.3.0-1645360070.exe

My current setup still using outdated WHS Version 1. I see from the nightly builds that the WHS version has not built since 20200216. Is WHS support being discontinued?

When fix is back-ported to 8.2, I can test there presuming that WHS builds are still working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants