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

Make powerOnTemperature optional #305

Merged
merged 1 commit into from
Jun 4, 2021
Merged

Make powerOnTemperature optional #305

merged 1 commit into from
Jun 4, 2021

Conversation

sindrebroch
Copy link
Contributor

Elgato Light Strip does not have the temperature-property, and thus can not load the component.

Proposed Changes

(Describe the changes and rationale behind them)

Related Issues

(Github link to related issues or pull requests)

Elgato Light Strip does not have the temperature-property, and thus can not load the component.
@sindrebroch
Copy link
Contributor Author

I haven't tested the changes because I don't have a proper dev-environment setup, and can't verify for other products other than Elgato Light Strip. But I got an error in HomeAssistant when the library tried to load the property, so I assume this fixes it.

@frenck
Copy link
Owner

frenck commented Jun 4, 2021

Hmmm, interesting, which firmware version are you running on the led strip?

As mine does have it:

image

(Nevertheless, we can make it optional, so at least it won't break 👍 )

@frenck frenck added the bugfix Inconsistencies or issues which will cause a problem for users or implementers. label Jun 4, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #305 (119c512) into main (c84beaa) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #305   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          133       133           
  Branches        18        18           
=========================================
  Hits           133       133           
Impacted Files Coverage Δ
src/elgato/models.py 100.00% <100.00%> (ø)

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 c84beaa...119c512. Read the comment docs.

@sindrebroch
Copy link
Contributor Author

Weird. Mine is running firmware version 1.0.4 (211).

image

@frenck
Copy link
Owner

frenck commented Jun 4, 2021

Hmm, it seems that it has to do with the mode the strip is in. The strip actually supports being in a color temperature mode (not exposed in the interface provided by Elgato).

So this change helps 👍

@frenck
Copy link
Owner

frenck commented Jun 4, 2021

Thanks!

@frenck frenck merged commit 70636b8 into frenck:main Jun 4, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2021
@sindrebroch sindrebroch deleted the patch-1 branch June 7, 2021 08:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix Inconsistencies or issues which will cause a problem for users or implementers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants