Brightness conversion for Abode dimmers by shred86 · Pull Request #13711 · home-assistant/core

shred86 · 2018-04-06T02:51:05Z

With AbodePy 0.12.3, dimmers will now work but a conversion of the brightness is required. Additionally, when a brightness value of 100 is sent to Abode, 99 is returned causing AbodePy to throw an error so this component will send 99 instead of 100.

Description:

Related issue (if applicable): N/A

Pull request in home-assistant.github.io with documentation (if applicable): N/A

Example entry for `configuration.yaml` (if applicable):

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable Update AbodePy version to 0.12.3 #13709

With AbodePy 0.12.3, dimmers will now work but a conversion of the brightness is required. Additionally, when a brightness value of 100 is sent to Abode, 99 is returned causing AbodePy to throw an error so this component will send 99 instead of 100.

homeassistant · 2018-04-06T02:51:06Z

Hi @shred86,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

syssi · 2018-04-06T05:45:11Z

homeassistant/components/light/abode.py

        if ATTR_BRIGHTNESS in kwargs and self._device.is_dimmable:
-            self._device.set_level(kwargs[ATTR_BRIGHTNESS])
+            # Convert HASS brightness (0-255) to Abode brightness (0-100)
+            brightness = int(kwargs[ATTR_BRIGHTNESS] * 100 / 255)


If 99 is the upper limit the range should be mapped to 0...99.

It's possible to pass a value of 100 to Abode but for some reason in the response it returns 99, so AbodePy throws an error when the passed value != return value. I'm new to Python but would something like this be a better solution?

brightness = int(max(min(kwargs[ATTR_BRIGHTNESS] * 100 / 255, 99), 0))

It's just math: kwargs[ATTR_BRIGHTNESS] provides a value between 0 and 255:

0 * 100 / 255 = 0 128 * 100 / 255 = int(50.19) = 50 255 * 100 / 255 = 100 vs. 0 * 99 / 255 = 0 128 * 99 / 255 = int(49.69) = 49 255 * 99 / 255 = 99

Checkout this one: #12203

It's not exactly your case but provides some support may be.

Lol, very good point. ;)

Ok, I think this is a much better implementation which also incorporates the solution you linked to. Using the ceil function from the math module:

return ceil(int(self._device.brightness) * 255 / 100)

Could you check another edge case? Is the light turned off if you set a brightness of 0 or is 0 the lowest brightness?

When I set the brightness all the way to 0, the light is turned off.

syssi · 2018-04-06T05:47:44Z

homeassistant/components/light/abode.py

        if self._device.is_dimmable and self._device.has_brightness:
-            return self._device.brightness
+            # Convert Abode brightness (0-100) to HASS brightness (0-255)
+            return int(self._device.brightness) * 255 / 100


Does the device return 100% ever here? I assume the upper limit should be 99 again.

The max value returned when setting the brightness is 99, but when HA first boots up and gets all of the devices from Abode, it will return 100 in this case.... so for this one I think we'll have to do something like:

return ceil(int(self._device.brightness) * 255 / 100)

But the "issue" now is there are times the return value can be off by 1 from the value that was sent (since we're dividing by 100 and not 99). Not really sure if it's an issue though.

Better implementation of the brightness conversion as it's less code and also provides a better UI experience as very small brightness values (i.e. 1, 2) won't be converted to 0.

Minor changes to pass checks and update to comments

syssi · 2018-04-06T08:05:49Z

homeassistant/components/light/abode.py

-            return self._device.brightness
+            # Convert Abode brightness (0-100) to HASS brightness (0-255)
+            # Abode will return 100 during initialization of Abode
+            return ceil(int(self._device.brightness) * 255 / 100)


99 * 255 / 100 = 252.45. It should be 255! What about brightness * 255 / 99

Just realized your comment. Hmmm...

Yeah I had it set to divide by 99 but noticed after restarting HA, Abode was returning a brightness value of 100 from one of my dimmers that was on which caused it to convert to 257. When you set the brightness to 100 is when it will only return 99.

In this case I would remap the brightness of 100:

brightness = int(self._device.brightness) if brightness == 100: return 255 else return ceil(brightness * 255 / 99)

Keeps the brightness value sent and returned from the device response consistent. However, during initialization and when a device refresh is received, Abode can return 100 thus we'll convert that case back to 99.

shred86 · 2018-04-06T15:16:28Z

@syssi - Thanks for the help! Just made another update to the brightness conversion and tested on my local HA install. Everything is working as expected.

If this is good to go, recommend we add it with #13709 which has already been approved. Otherwise, brightness functionality is not going to work properly without this conversion for Abode dimmers.

syssi · 2018-04-06T19:51:42Z

Could you fix this lint issue: R: 75,12: Unnecessary "else" after "return" (no-else-return)

syssi · 2018-04-06T19:57:33Z

homeassistant/components/light/abode.py

-            self._device.set_level(kwargs[ATTR_BRIGHTNESS])
+            # Convert HASS brightness (0-255) to Abode brightness (0-99)
+            # If 100 is sent to Abode, response is 99 causing an error
+            self._device.set_level(ceil(kwargs[ATTR_BRIGHTNESS] * 99 / 255))


Please device by 255.0:

>>> ceil(1 * 99 / 255) 0.0 >>> ceil(1 * 99 / 255.0) 1.0

syssi · 2018-04-06T20:02:02Z

homeassistant/components/light/abode.py

+                return 255
+            else:
+                # Convert Abode brightness (0-99) to HASS brightness (0-255)
+                return ceil(brightness * 255 / 99)


With AbodePy 0.12.3, dimmers will now work but a conversion of the brightness is required. Additionally, when a brightness value of 100 is sent to Abode, 99 is returned causing AbodePy to throw an error so this component will send 99 instead of 100. Keeps the brightness value sent and returned from the device response consistent. However, during initialization and when a device refresh is received, Abode can return 100 thus we'll convert that case back to 99.

homeassistant added cla-needed platform: light.abode small-pr PRs with less than 30 lines. cla-signed and removed cla-needed labels Apr 6, 2018

shred86 mentioned this pull request Apr 6, 2018

Dimmer MisterWil/abodepy#22

Closed

syssi reviewed Apr 6, 2018

View reviewed changes

Update abode.py

1d0a28a

Better implementation of the brightness conversion as it's less code and also provides a better UI experience as very small brightness values (i.e. 1, 2) won't be converted to 0.

homeassistant added the cla-signed label Apr 6, 2018

Update abode.py

7ddc23b

Minor changes to pass checks and update to comments

syssi reviewed Apr 6, 2018

View reviewed changes

Updates conversion of brightness

fac439b

Keeps the brightness value sent and returned from the device response consistent. However, during initialization and when a device refresh is received, Abode can return 100 thus we'll convert that case back to 99.

syssi reviewed Apr 6, 2018

View reviewed changes

Minor updates to conversion

90f1981

homeassistant added the cla-signed label Apr 6, 2018

syssi merged commit 2bf17cb into home-assistant:dev Apr 7, 2018

fabaff mentioned this pull request Apr 7, 2018

Reset permission #13743

Merged

2 tasks

balloob mentioned this pull request Apr 27, 2018

0.68 #14124

Merged

home-assistant locked and limited conversation to collaborators Jul 26, 2018

ghost added integration: abode and removed platform: light.abode labels Mar 21, 2019

Uh oh!

Conversation

shred86 commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

homeassistant commented Apr 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shred86 Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shred86 Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shred86 Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shred86 commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

syssi commented Apr 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

shred86 commented Apr 6, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

shred86 Apr 6, 2018 •

edited

Loading

shred86 Apr 6, 2018 •

edited

Loading

shred86 Apr 6, 2018 •

edited

Loading

shred86 commented Apr 6, 2018 •

edited

Loading