Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 a max brightness field for each dimmer in config file #149 #200
Add a max brightness field for each dimmer in config file #149 #200
Changes from 11 commits
08f2236
2e92194
eac49b0
9c88c38
0675f75
52c2b14
58487c3
dc89a94
1559b0b
66c18fe
a271ede
1f4b415
f81a8b4
cdd44db
8065695
d729345
f591f0b
418c2d1
df1f3ba
e986fef
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some warnings here, could you please take a look at those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data?.max_brightness
will not be defined in case the user does not enter device defaults information.Please use the old value (255) as the default value In this case.
This will use 255 as fallback incase the
max_brightness
is not set.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I set the default value for max brightness to be 255 in the config file, then I read that value weather its provided or not, good enough? (check out latests commit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you only set the "default" value in the ui config. This has nothing to do with the actual config file. Just when users use config-ui-x. Furthermore they can still remove the default value, making it undefined. Or not add the device config at all. Therefor I believe the proposed changes are still necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data?.max_brightness is null for me, before the change I read the value from the config file, where is the connection between data.max_brightness and config.max_brightness should be set?