-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[surepetcare] Add support for waterstation Felaqua #18114
base: main
Are you sure you want to change the base?
Conversation
* Fixes: openhab#15654 * added Felaqua pet waterstation * added UoM where possible * other small fixes Signed-off-by: Holger Eisold <[email protected]>
if someone wants to test on a 4.3.x install.. please let me know if there are any issues |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/new-sure-petcare-binding-for-cat-pet-flap/82257/91 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding Felaqua support, fixing some old inconsistencies and extending documentation as well! I have added a few comments, otherwise LGTM.
@@ -122,4 +124,7 @@ public class SurePetcareConstants { | |||
public static final String PET_CHANNEL_FEEDER_LAST_CHANGE = "feederLastChange"; | |||
public static final String PET_CHANNEL_FEEDER_LAST_CHANGE_LEFT = "feederLastChangeLeft"; | |||
public static final String PET_CHANNEL_FEEDER_LAST_CHANGE_RIGHT = "feederLastChangeRight"; | |||
public static final String PET_CHANNEL_WATER_DEVICE = "waterDevice"; | |||
public static final String PET_CHANNEL_WATER_LASTDRINKING = "waterLastDrinking"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static final String PET_CHANNEL_WATER_LASTDRINKING = "waterLastDrinking"; | |
public static final String PET_CHANNEL_WATER_LAST_DRINKING = "waterLastDrinking"; |
<property name="productType"/> | ||
<property name="productName"/> | ||
<property name="pairingAt"/> | ||
<property name="thingTypeVersion">1</property> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not needed (yet).
<thing-type uid="surepetcare:waterDevice"> | ||
<instruction-set targetVersion="1"> | ||
<update-channel id="deviceRSSI"> | ||
<type>surepetcare:rssiDeviceType</type> | ||
</update-channel> | ||
<update-channel id="hubRSSI"> | ||
<type>surepetcare:rssiHubType</type> | ||
</update-channel> | ||
</instruction-set> | ||
</thing-type> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the thing type waterDevice
is introduced with this PR, no update instructions are needed.
<thing-type uid="surepetcare:waterDevice"> | |
<instruction-set targetVersion="1"> | |
<update-channel id="deviceRSSI"> | |
<type>surepetcare:rssiDeviceType</type> | |
</update-channel> | |
<update-channel id="hubRSSI"> | |
<type>surepetcare:rssiHubType</type> | |
</update-channel> | |
</instruction-set> | |
</thing-type> |
However, for thing type pet
you have introduced three new channels, and they need to be added by update instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert changes in this file. Translations should be provided through Crowdin after the PR is merged. See https://www.openhab.org/docs/developer/utils/i18n.html#managing-translations
@@ -175,6 +177,8 @@ protected void updateThing() { | |||
new StringType(device.control.lid.closeDelayId.toString())); | |||
updateState(DEVICE_CHANNEL_BOWLS_TRAINING_MODE, | |||
new StringType(device.control.trainingModeId.toString())); | |||
} else if (thing.getThingTypeUID().equals(THING_TYPE_WATER_DEVICE)) { | |||
// TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something missing?
Thing pet 12345 "My Cat" @ "SurePetcare Pets" | ||
} | ||
``` | ||
|
||
### Items configuration | ||
|
||
<details> | ||
<summary>New Items with Points (semantic model)</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is any reason to keep the "old" version if the only difference is that tags are added, and the example is extended?
|
||
// Equipment representing thing: | ||
// surepetcare:household:api:CHANGE_ME | ||
// (SurePetcare Haushalt EddyMurphy) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example should be in English - please check all item names, labels etc.
Thing pet 12345 "My Cat" @ "SurePetcare Pets" | ||
} | ||
``` | ||
|
||
### Items configuration | ||
|
||
<details> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this will work, but we can give it a try. Did you see other binding README's use this?
Signed-off-by: Holger Eisold [email protected]