Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions homeassistant/components/light/abode.py
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ def turn_on(self, **kwargs):
*kwargs[ATTR_HS_COLOR]))

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@shred86 shred86 Apr 6, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checkout this one: #12203

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

@shred86 shred86 Apr 6, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

# If 100 is sent to Abode, 99 is returned causing an error
if brightness == 100:
brightness = 99
self._device.set_level(brightness)
else:
self._device.switch_on()

Expand All @@ -68,7 +73,8 @@ def is_on(self):
def brightness(self):
"""Return the brightness of the light."""
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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@shred86 shred86 Apr 6, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


@property
def hs_color(self):
Expand Down