amimof / huego

An extensive Philips Hue client library for Go with an emphasis on simplicity

MIT License

250 stars 36 forks source link

Add ability to read, and set the active state of streams on groups #29

Closed SteelPhase closed 3 years ago

SteelPhase commented 4 years ago

This is exposing the RESTful portion of the Entertainment API

codecov-io commented 4 years ago

Codecov Report

Merging #29 into master will decrease coverage by 0.50%. The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
- Coverage   67.28%   66.78%   -0.51%     
==========================================
  Files           5        5              
  Lines        1131     1165      +34     
==========================================
+ Hits          761      778      +17     
- Misses        202      214      +12     
- Partials      168      173       +5

Impacted Files	Coverage Δ
group.go	`86.40% <50.00%> (-13.60%)`	:arrow_down:

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 7b16d16...648a98c. Read the comment docs.

amimof commented 4 years ago

@SteelPhase Thanks for the contribution. I was thinking. Perhaps it’s better to name the functions EnableStreaming() and DisableStreaming() for readability. What do you think?

SteelPhase commented 4 years ago

That's a good Name. I was undecided on what to call it and just picked something. I'll try it out.

SteelPhase commented 4 years ago

Sorry for the delay. I adjust the naming, and tested further. Found a few bugs with how nil booleans were handled in the DisableStream function. Verified it's all working as it should.

SteelPhase commented 4 years ago

Going to need a bit of time to look at the tests, but i'll verify what I can soon

SteelPhase commented 4 years ago

I added significantly more testing around the entertainment api components

SteelPhase commented 4 years ago

Got sick of fighting with the unit tests differences between 1.13 and 1.14, I split them out. I'm waiting for the travis-ci job to complete.

SteelPhase commented 4 years ago

I think this is ready for review again

amimof commented 4 years ago

@SteelPhase You might have already tried it but perhaps you could use the assert? For example:

_, err := b.getAPIPath("/")
assert.NotNil(t, err)

SteelPhase commented 4 years ago

Switched it over to use assert.NotNil

SteelPhase commented 3 years ago

Should be fixed. got distracted with GopherCon stuff