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

[Tradfri] Added FLOALT panels #4366

Merged
merged 6 commits into from Oct 3, 2017

Conversation

Projects
None yet
3 participants
@cweitkamp
Copy link
Contributor

commented Oct 2, 2017

  • Added "FLOALT" panels and "TRADFRI bulb E27 WS clear 950lm" to COLOR_TEMP_MODELS
  • Added representation-property to thing types
  • Refactored xml files and applied formatter on xml files
  • Changed spelling of TRÅDFRI throughout the whole binding

After you added a FLOALT panel to your environment it is correclty discovered as 0220 thing type. Once it becomes not reachable it is additionally discovered as 0100 thing type. This fix should prevent this behavior.

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

Added FLOALT panels to COLOR_TEMP_MODELS
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

Thanks, great to see support for the FLOALT devices.

The behavior worries me a bit though: We will see the same for every new upcoming device that we do not yet specifically support. Shouldn't we make sure that we do not add additional discovery results? The recently introduced representation-property of things might probably help here. Would you want to have a look into that?

cweitkamp added some commits Oct 3, 2017

Added representation-property; Refactored xml files;
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Applied formatter on xml files;
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Added LED-bulb E27 950 lm to COLOR_TEMP_MODELS
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

I added the representation-property to the thing type definition. But I am not sure if this is the solution for the real problem (see comment in TradfriDiscoveryService#82).

@kaikreuzer
Copy link
Member

left a comment

But I am not sure if this is the solution for the real problem (see comment in TradfriDiscoveryService#82).

Hm, this comments seems to be lost, at least I cannot see it when following this link...

xsi:schemaLocation="http://eclipse.org/smarthome/schemas/config-description/v1.0.0 http://eclipse.org/smarthome/schemas/config-description-1.0.0.xsd">

<config-description uri="thing-type:tradfri:device">
<parameter-group name="device">

This comment has been minimized.

Copy link
@kaikreuzer

kaikreuzer Oct 3, 2017

Member

Please don't use parameter groups if there is just a single parameter to put into it...

This comment has been minimized.

Copy link
@cweitkamp

cweitkamp Oct 3, 2017

Author Contributor

No problem. I removed it.

</thing-type>

<!-- note that this isn't yet supported by the code as we do not receive any data
from the gateway for it -->

This comment has been minimized.

Copy link
@kaikreuzer

kaikreuzer Oct 3, 2017

Member

why is here a new line?

@cweitkamp

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

I am sorry. Maybe it got lost after a forced push: TradfriDiscoveryService.java#L82.

@kaikreuzer

This comment has been minimized.

Copy link
Member

commented Oct 3, 2017

Ok, you are right with the comment. If we do discovery while the bulb is unreachable, we will actually discover the wrong (or let's say a too simple) type. But the representation-property should nonetheless avoid that we see a new discovery result. So if the Thing has the wrong type, the user can delete it and re-discover it with the new type.

@cweitkamp

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

Seems to work very well. I put a version similar to this PR - without the COLOR_TEMP_MODELS check - into my prod environment yesterday. The discovery shows an INFO logging for new devices with wrong thing type from time to time but nothing pops up in my Paper UI inbox. Should we maybe remove it completely? wdyt?

cweitkamp added some commits Oct 3, 2017

Changed spelling of TRÅDFRI
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Applied changes from review
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@kaikreuzer
Copy link
Member

left a comment

Thanks!

@kaikreuzer kaikreuzer merged commit 1e308ce into eclipse:master Oct 3, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
ip-validation
Details

@cweitkamp cweitkamp deleted the cweitkamp:feature-tradfri-devices branch Oct 3, 2017

@hreichert

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2017

Side note: All IKEA products got a common identifier in their name. I used this in #4271 to distinguish between color lights and spectrum lights.

  • " W " -> dimmable only
  • " WS " -> white spectrum
  • " CWS " -> color white spectrum

Maybe this is a more future-safe approach to detect the type?

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.