MariusRumpf / node-lifx

Node.js implementation of the LIFX LAN protocol :bulb:
MIT License
144 stars 28 forks source link

Infrared support for new LIFX+ (rebased to develop) #52

Closed ristomatti closed 7 years ago

ristomatti commented 7 years ago

Here's the pull request #42 by @bmv437 rebased to develop as discussed in Gitter.

I have tested the IR support with lifxsh. The feature is included on a dev branch here https://github.com/ristomatti/lifxsh/tree/ir-brightness if someone wants a quick way to try the changes out.

codecov-io commented 7 years ago

Current coverage is 59.98% (diff: 59.01%)

Merging #52 into develop will decrease coverage by 0.04%

@@            develop        #52   diff @@
==========================================
  Files            42         45     +3   
  Lines          1346       1407    +61   
  Methods         111        118     +7   
  Messages          0          0          
  Branches        216        223     +7   
==========================================
+ Hits            808        844    +36   
- Misses          538        563    +25   
  Partials          0          0          

Powered by Codecov. Last update 60ad066...c64e3b8

codecov-io commented 7 years ago

Current coverage is 59.98% (diff: 59.01%)

Merging #52 into develop will decrease coverage by 0.04%

@@            develop        #52   diff @@
==========================================
  Files            42         45     +3   
  Lines          1346       1407    +61   
  Methods         111        118     +7   
  Messages          0          0          
  Branches        216        223     +7   
==========================================
+ Hits            808        844    +36   
- Misses          538        563    +25   
  Partials          0          0          

Powered by Codecov. Last update 60ad066...c64e3b8

ristomatti commented 7 years ago

@MariusRumpf Two checks fail which did not fail on the original one. If I understand correctly, the code coverage has improved just enough on the dev branch to make the coverage fail on this one. Can this still be merged or should I just close this?