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.

Sign up for GitHub

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

[netatmo] Semantics added to channel types #10973

Closed
wants to merge 4 commits into from
Closed

[netatmo] Semantics added to channel types #10973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e4027c1
[netatmo] Semantics added to channel types
lolodomo Jul 11, 2021
303aae4
Review comment: tag "Light" added for floodlight
lolodomo Jul 12, 2021
9e8d7a3
Review comment: tags updated for channel person_athome
lolodomo Jul 12, 2021
66962ed
Keep only obvious tags (with an identified property)
lolodomo Jul 20, 2021
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
16 changes: 16 additions & 0 deletions bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/thing/camera.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
<item-type>Switch</item-type>
<label>State</label>
<description>State of the camera</description>
<tags>
<tag>Switch</tag>
<tag>Power</tag>
</tags>
</channel-type>

<channel-type id="sd_status">
Expand All @@ -69,6 +73,10 @@
<item-type>Switch</item-type>
<label>Alim State</label>
<description>State of the power connector</description>
<tags>
<tag>Status</tag>
<tag>Power</tag>
</tags>
<state readOnly="true"></state>
</channel-type>

Expand Down Expand Up @@ -105,12 +113,20 @@
<item-type>Switch</item-type>
<label>Floodlight Auto-Mode</label>
<description>State of the floodlight auto-mode</description>
<tags>
<tag>Control</tag>
<tag>Light</tag>
</tags>
</channel-type>

<channel-type id="floodlight">
<item-type>Switch</item-type>
<label>Floodlight</label>
<description>State of the floodlight</description>
<tags>
<tag>Switch</tag>
Copy link
Member

Choose a reason for hiding this comment

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

Can you help me understand what the "Switch" tag was good for? This channel seems to be related to light, so I'd rather would have expected a tag for that. Just because something is "switchable" is imho not enough for such a tag, especially as this information is already contained in the item-type.

Copy link
Contributor

@clinique clinique Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

To tell the truth @kaikreuzer , I did not spend much time reviewing this because existing version of the Netatmo binding is hardly functional due to some evolutions on Netatmo API side. I rewrote it completely (opened PR on this) still WIP but 95% done. Heavy work ongoing. I expect to be able to target OH 3.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree, "Light" should be added.

Copy link
Member

Choose a reason for hiding this comment

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

But then again, "Light" is usually used to light up some room. A floodlight of a camera has a fairly special semantic and shouldn't be used as a random light (you don't want it to go on when you enter the room. You probably do not want it to turn off, when you're leaving the house, etc.)
I'd in such cases hence rather prefer to not put any tag on it, but leave it to the user on how he actually uses the device. Only he knows his concrete use cases.

Copy link
Member

Choose a reason for hiding this comment

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

Note that there a few other "Switch" tags in that PR as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just look at Netatmo product information, the outside camera integrates a real light, it combines a light and a camera.
https://www.netatmo.com/fr-fr/security/cam-outdoor
Considering it as a light that can be switch on and off is IMHO fully correct as it is what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked again all channels with only "Switch" tag and fixed them.

<tag>Light</tag>
</tags>
</channel-type>

</thing:thing-descriptions>
Loading