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

fix: fix multiple parameters for hass Discovery #36

Merged
merged 17 commits into from
Nov 30, 2020
Merged

fix: fix multiple parameters for hass Discovery #36

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1c08edd
Fix Undefined property when no label exposed by Driver
varet80 Nov 28, 2020
1b9b564
Add battery_level unit of measurement
varet80 Nov 28, 2020
4bb2601
add patch for valueId.unit
varet80 Nov 28, 2020
0a51bd6
add more changes on valueId.unit
varet80 Nov 28, 2020
959ce9d
Fix binary sensor discovery
varet80 Nov 29, 2020
d3a0ba0
remove import of BinarySensorType, as is not used
varet80 Nov 29, 2020
8fcf838
fix a linting issue
varet80 Nov 29, 2020
2123d7f
Update lib/Gateway.js
varet80 Nov 29, 2020
181dc85
remove valueId.units, as not existing
aretakisv Nov 30, 2020
b015703
add an if
aretakisv Nov 30, 2020
62c7e44
remove if on valueId.property
aretakisv Nov 30, 2020
fba19bc
add if on valueId.unit, by mistake added it on valueId.property
aretakisv Nov 30, 2020
3fc1d64
add prettier ignore for README.md
aretakisv Nov 30, 2020
a6823cb
Merge branch 'feat#hassDiscovery' into fix#uiUndefinedProperty
aretakisv Nov 30, 2020
e8d9040
add property prefix for Metadatalabels not discovered
aretakisv Nov 30, 2020
24e79fb
fix linting on property patch
aretakisv Nov 30, 2020
89cc79f
fix: default label
robertsLando Nov 30, 2020
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
21 changes: 12 additions & 9 deletions lib/Gateway.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const path = require('path')
const reqlib = require('app-root-path').require
const utils = reqlib('/lib/utils.js')
const EventEmitter = require('events')
const { AlarmSensorType, BinarySensorType } = require('zwave-js')
const { AlarmSensorType } = require('zwave-js')
const { CommandClasses } = require('@zwave-js/core')
const Constants = reqlib('/lib/Constants.js')
const debug = reqlib('/lib/debug')('Gateway')
Expand Down Expand Up @@ -1265,17 +1265,21 @@ Gateway.prototype.discoverValue = function (node, vId) {
case CommandClasses['Binary Sensor']:
// https://github.com/zwave-js/node-zwave-js/blob/master/packages/zwave-js/src/lib/commandclass/BinarySensorCC.ts#L41

let sensorTypeName = BinarySensorType[valueId.property]
// change the sensorTypeName to use directly valueId.property, as the old way was returning a number
// add a comment which shows the old way of achieving this value. This change fixes the Binary Sensor
// discovery
let sensorTypeName = valueId.property
robertsLando marked this conversation as resolved.
Show resolved Hide resolved
// unsued discovery line. for historical reasons
// let sensorTypeId = BinarySensorType[valueId.property]

if (sensorTypeName) {
sensorTypeName = this.mqtt.cleanName(
sensorTypeName.toLocaleLowerCase()
)
}

// TODO: Implement all BinarySensorTypes
cfg = hassCfg['binary_sensor_' + sensorTypeName]

// if cannot discover anything, assume contact type
cfg = cfg ? copy(cfg) : copy(hassCfg.binary_sensor_contact)

if (valueConf) {
Expand Down Expand Up @@ -1349,7 +1353,8 @@ Gateway.prototype.discoverValue = function (node, vId) {
sensor: 'battery',
objectId: 'level',
props: {
device_class: 'battery'
device_class: 'battery',
unit_of_measurement: '%' // this is set if Driver doesn't offer unit of measurement
}
}
} else return
Expand All @@ -1361,10 +1366,8 @@ Gateway.prototype.discoverValue = function (node, vId) {

Object.assign(cfg.discovery_payload, sensor.props || {})

if (valueId.units) {
// https://github.com/zwave-js/node-zwave-js/blob/master/packages/config/config/scales.json
cfg.discovery_payload.unit_of_measurement = valueId.units
}
// https://github.com/zwave-js/node-zwave-js/blob/master/packages/config/config/scales.json
cfg.discovery_payload.unit_of_measurement = valueId.unit
varet80 marked this conversation as resolved.
Show resolved Hide resolved

// check if there is a custom value configuration for this valueID
if (valueConf) {
Expand Down
2 changes: 1 addition & 1 deletion lib/ZwaveClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ function updateValueMetadata (zwaveNode, zwaveValue, zwaveValueMeta) {
readable: zwaveValueMeta.readable,
writeable: zwaveValueMeta.writeable,
description: zwaveValueMeta.description,
label: zwaveValueMeta.label,
label: zwaveValueMeta.label || zwaveValue.propertyName, // when label is missing, re use propertyName. Usefull for webinterface
Copy link
Member

Choose a reason for hiding this comment

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

@AlCalzone could the label be undefined or is this caused by an error on zwavejs? If it's the second case I would remove this so we can detect undefined labels and fix them on zwavejs

Copy link
Member

Choose a reason for hiding this comment

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

The Label can be undefined if we don't set it in zwave-js. Your code should be prepared for that possibility.
But as I wrote in slack, we should find those locations and eliminate them.

Copy link
Member

@robertsLando robertsLando Nov 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ok so in poor words should I keep this change or remove this so we can detect undefined labels?

I think it's a good fix but we would never see the undefined labels if I keep this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we could have a suffix/prefix Property and help us still know what is, but not let it undifined
something like

label: zwaveValueMeta.label || ("%s - property", zwaveValue.propertyName),

Copy link
Member

Choose a reason for hiding this comment

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

This could be another valid solution

Copy link
Collaborator Author

@varet80 varet80 Nov 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

i think more beautifull would be:

label: zwaveValueMeta.label || ("%s (property)", zwaveValue.propertyName),

pushing this as change. If you prefer different approach. please comment here

default: zwaveValueMeta.default
}

Expand Down