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

Correct hue light level units and value calculation #23309

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

mitchellrj
Copy link
Contributor

As pointed out by #22598 (comment)

@ghost
Copy link

ghost commented Apr 22, 2019

Hey there @balloob, mind taking a look at this pull request as its been labeled with a integration (hue) you are listed as a codeowner for? Thanks!

This is a automatic comment generated by codeowners-mention to help ensure issues and pull requests are seen by the right people.

@mitchellrj mitchellrj force-pushed the add-basic-hue-sensor-support branch from 4c03f0d to e59870d Compare April 22, 2019 19:53
@balloob balloob added this to the 0.92.0 milestone Apr 23, 2019
@balloob balloob merged commit 845d81b into home-assistant:dev Apr 23, 2019
@SeanPM5
Copy link
Contributor

SeanPM5 commented Apr 23, 2019

It seems that after updating from b2 to b3 the sensor values are extremely different, so I think there might be a minor bug with the calculation/rounding here.

It used to report easy to read values like 3847 and now it's very long numbers that do not appear rounded 46.838178785024404 lx.

Here is a quick screen recording demonstrating the difference when scrolling through values on the more-info window: https://gfycat.com/ThreadbareUnfinishedGnatcatcher

@mitchellrj
Copy link
Contributor Author

mitchellrj commented Apr 23, 2019 via email

@Mariusthvdb
Copy link
Contributor

Mariusthvdb commented May 5, 2019

not sure if this is still allowed here, but the new light_level sensors are displaying lx, and not the native light_level off of the Hue hub. Might be better calling them likewise, sensor.X_sensor_lx.

Feel with @SeanPM5 on the rounding, which could be a bit less precise (put differently, please round to max 2 decimals). There's really no use case for more precision, even at low lightlevels, and it either forces users to have horrible UI, or create extra template sensors to mitigate that... compare this:

Schermafbeelding 2019-05-05 om 23 50 11

to:

Schermafbeelding 2019-05-05 om 23 50 18

community-post: https://community.home-assistant.io/t/new-hue-light-level-sensors-are-in-fact-lux-sensors/115112

this is Philips own light_level table and shows no use for these light_level detailing:

#Philips Hue definition                 Lux               MeasuredValue
#Overcast moonless night sky              0.0001                  0
#Outdoor: Bright moonlight                1                       1
#Home: Night light                        2                    3000
#Home: Dimmed light                      10                   10000
#Home: ‘Cosy’ living room                50                   17000
#Home: ‘Normal’ non-task light          150                   22000
#Home: Working / reading                350                   25500
#Home: Inside daylight                  700                   28500
#Home: Maximum to avoid glare          2000                   33000
#Outdoor: Clear daylight            > 10000                 > 40000
#Outdoor: direct sunlight            120000                   51000

@MartinHjelmare
Copy link
Member

Please open an issue if you suspect a bug.

If you want to suggest an enhancement please open a feature request in the Feature Requests section of our community forum.

Merged PRs should not be used for enhancement discussion or bug reports. If you've found a bug it's ok to make a review with inline comments and link to an issue that reports the bug.

Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators May 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants