[Main UI] Added Badge for BatteryLow warning #1063
Conversation
Signed-off-by: Christoph Weitkamp <[email protected]>
| @@ -70,6 +71,10 @@ export default { | |||
| query () { | |||
| let direct, equipment, allPoints, points | |||
| switch (this.type) { | |||
| case 'battery': | |||
| direct = findPoints(this.element.properties, 'Point_LowBattery', true) | |||
There was a problem hiding this comment.
Should we check for property "Energy" too?
| direct = findPoints(this.element.properties, 'Point_LowBattery', true) | |
| direct = findPoints(this.element.properties, 'Point_LowBattery', true, 'Property_Energy') |
There was a problem hiding this comment.
Should we check for property "Energy" too?
I think that's superfluous since the point class alone is self-describing.
Also it's Point_Status_LowBattery, not Point_LowBattery.
Job #129: Bundle Size — 10.41MB (~+0.01%). |
There was a problem hiding this comment.
Thanks, yes basically that's it - you got the class name wrong though and I'd suggest another icon.
The code of these status badges could definitely be improved to be more generic and extensible, so people could add their own badges (the code to map related items is rather similar for each types).
Perhaps a check on BatteryLevel points with a numerical state below 10 % could be also considered since this is what I usually see as channels, never "low battery" explicitely. Though probably LowBattery items could be linked to the actual level with a 0-10 Range profile...
| @@ -70,6 +71,10 @@ export default { | |||
| query () { | |||
| let direct, equipment, allPoints, points | |||
| switch (this.type) { | |||
| case 'battery': | |||
| direct = findPoints(this.element.properties, 'Point_LowBattery', true) | |||
There was a problem hiding this comment.
Should we check for property "Energy" too?
I think that's superfluous since the point class alone is self-describing.
Also it's Point_Status_LowBattery, not Point_LowBattery.
bundles/org.openhab.ui/web/src/components/cards/glance/location/status-badge.vue
Outdated
Show resolved
Hide resolved
Signed-off-by: Christoph Weitkamp <[email protected]>
|
Thanks for your feedback. I changed the class name and the icon. I finally got it running in my own dev environment. It would be a nice enhancement if users can define their own Badges or change the existing logic. I already have another improvement in my mind. I would like to consider equipment of type "VoiceAssitant" and state "PLAY" for speakers. I am no friend of fixed values. e.g. I would prefer 15% for Battery Low. Thus I think the Profiles Hysteresis and Range are the best solutions. |
There was a problem hiding this comment.
Yes, the purpose of these badges wasn't originally designed with user extensibility in mind, rather provide sensible hardcoded defaults for common equipment so that stuff appears automatically, but of course it was a poor choice since the OH users like customizability, and since it proves quite popular and needs arise we can revisit this goal.
That's also why I mentioned the "battery level < 10%" as a suggestion as an additional criteria for bringing up the "battery low" icon, as many devices only have battery level channels that are likely to be mapped as-is with the default profile, and few users would bother to define dedicated "battery low" items with profiles for their equipment (or even know how to) if it's not offered as a channel.
But for now let's just agree on the BatteryLow point requirement 👍
| case 'battery': | ||
| direct = findPoints(this.element.properties, 'Point_Status_LowBattery', true) | ||
| if (direct.length) return direct | ||
| return findPoints(allEquipmentPoints(this.element.equipment), 'Point_LowBattery', true) |
There was a problem hiding this comment.
You missed this one!
| return findPoints(allEquipmentPoints(this.element.equipment), 'Point_LowBattery', true) | |
| return findPoints(allEquipmentPoints(this.element.equipment), 'Point_Status_LowBattery', true) |
There was a problem hiding this comment.
🤦🏼 Ups ... changed it.
Signed-off-by: Christoph Weitkamp <[email protected]>
| @@ -70,6 +71,10 @@ export default { | |||
| query () { | |||
| let direct, equipment, allPoints, points | |||
| switch (this.type) { | |||
| case 'battery': | |||
| direct = findPoints(this.element.properties, 'Point_Status_LowBattery', true) | |||
There was a problem hiding this comment.
I'm just realizing now that lines 75 & 76 here might even be unnecessary - these prioritize points directly under the Location over those under an Equipment... but having LowBattery points directly under a Location makes little sense I think, you would expect batteries to be under Equipment all the time?
Edit: Never mind, let's be open-minded, some locations might have batteries, whataver that means :)
|
@cweitkamp I think an update to the docs (https://www.openhab.org/docs/tutorial/model.html#badges) is in order - I wish these would be in a reference doc rather than the tutorial... |
|
Of course: openhab/openhab-docs#1583. |
Closes #1060
@ghys Untested new feature. I need your advice. Is it really that easy? Or did I miss something?
Signed-off-by: Christoph Weitkamp [email protected]