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 separate message queue for each address (device or broadcast) (base branch es2015) #60

Open
wants to merge 3 commits into
base: es2015
Choose a base branch
from

Conversation

ristomatti
Copy link
Collaborator

This modifies Client implementation to hold a separate message queue and send timer for each device or multicast address. This should give a noticeable speedup when sending packets to multiple devices.

The LIFX protocol spec defines the maximum message rate which is defined in constants.js as:

MESSAGE_RATE_LIMIT: 50, // in ms

However the number is the maximum rate to send for each device, not the rate of messages in the network. With having a separate message queue for each device we can achieve near immediate response in all lights when multiple lights are addressed at once.

@ristomatti
Copy link
Collaborator Author

@MariusRumpf I tried reaching you through Gitter but since I was not sure if you're following it these days, I decided to push my initial es2015 refactoring branch upstream. This PR is made using it as the base.

I see the CI stuff is failing, probably because the project needs to be built first. I'll see if I can do something about that but as I'm not familiar with any of the CI services I would appreaciate if you could help with this.

@ristomatti ristomatti force-pushed the es2015-multiple-message-queues branch from d1f54b2 to 06214ca Compare October 29, 2017 13:45
@codecov-io
Copy link

codecov-io commented Oct 29, 2017

Codecov Report

Merging #60 into es2015 will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           es2015      #60      +/-   ##
==========================================
+ Coverage   58.41%   58.74%   +0.33%     
==========================================
  Files          50       50              
  Lines        1688     1704      +16     
  Branches      255      259       +4     
==========================================
+ Hits          986     1001      +15     
- Misses        702      703       +1
Impacted Files Coverage Δ
lib/lifx/client.js 89.34% <0%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0468de6...02528d6. Read the comment docs.

@ristomatti
Copy link
Collaborator Author

Ok managed to fix the tests (pushed changes on the base branch). See commits a8f0fc5, efba035 and 90e9078.

@ristomatti ristomatti force-pushed the es2015-multiple-message-queues branch from 06214ca to c7bf72a Compare October 29, 2017 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants