fix: topics and hass ids/names fixes (#247)

* fix: sanitize slash in properties * fix: better id sanification * fix: lint errors * fix: other lint errors * fix: usage of propertyKey/name Fixes #239 #232 * fix: joinProps helper and sanitize object id * fix: move sanitifaction methods to utils * fix: lint issues * fix: sanification of numbers * fix: typo * fix: remove meter type from meters object id * fix: add missing toLowerCase * fix: add missing toLowerCase * fix: lint issues
zwave-js · Jan 21, 2021 · a072c73 · a072c73
1 parent 9a8073b
commit a072c73
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 66 deletions.
diff --git a/docs/usage/setup.md b/docs/usage/setup.md
@@ -165,6 +165,7 @@ Gateway settings:
   - `%ln`: Node location with name `<location-?><name>`
   - `%n`: Node Name
   - `%loc`: Node Location
+  - `%p`: valueId property (fallback to device type)
   - `%pk`: valueId property key (fallback to device type)
   - `%pn`: valueId property name (fallback to device type)
   - `%o`: HASS object_id

diff --git a/lib/Constants.js b/lib/Constants.js
@@ -23,8 +23,8 @@ module.exports = {
     )
 
     const cfg = {
-      sensor: (meter ? meter.name : 'unknown') + ccSpecific.meterType,
-      objectId: (scale ? scale.label : 'unknown' + ccSpecific.scale) + '_meter',
+      sensor: meter ? meter.name : 'unknown',
+      objectId: scale ? scale.label : 'unknown' + ccSpecific.scale,
       props: {}
     }
 

diff --git a/lib/Gateway.js b/lib/Gateway.js
@@ -331,7 +331,7 @@ function onNotification (node, notificationLabel, parameters) {
   const topic =
     this.nodeTopic(node) +
     '/notification/' +
-    this.mqtt.cleanName(notificationLabel)
+    utils.sanitizeTopic(notificationLabel, true)
   let data
 
   parameters = parameters ? parameters.toString() : null
@@ -567,15 +567,6 @@ function deviceInfo (node, nodeName) {
 function getDiscoveryTopic (hassDevice, nodeName) {
   return `${hassDevice.type}/${nodeName}/${hassDevice.object_id}/config`
 }
-/**
- * Sanitize ids removing chars that could break discovery
- *
- * @param {string} id The id string
- * @returns The sanitized id, lower cases and without spaces
- */
-function sanitizeId (id) {
-  return id.replace(/\s/g, '_').toLocaleLowerCase()
-}
 
 /**
  * Calculate the correct template string to use for modes templates
@@ -659,10 +650,12 @@ function getEntityName (node, valueId, cfg, entityTemplate, ignoreLoc) {
   // when getting the entity name of a device use node props
   let propertyKey = cfg.type
   let propertyName = cfg.type
+  let property = cfg.type
   let label = cfg.object_id
 
   if (valueId) {
-    propertyKey = valueId.propertyKey || valueId.propertyName
+    property = valueId.property
+    propertyKey = valueId.propertyKey
     propertyName = valueId.propertyName
     label = valueId.label
   }
@@ -673,6 +666,7 @@ function getEntityName (node, valueId, cfg, entityTemplate, ignoreLoc) {
     .replace(/%loc/g, node.loc || '')
     .replace(/%pk/g, propertyKey)
     .replace(/%pn/g, propertyName)
+    .replace(/%p/g, property)
     .replace(/%o/g, cfg.object_id)
     .replace(/%n/g, node.name || NODE_PREFIX + node.id)
     .replace(/%l/g, label)
@@ -828,7 +822,7 @@ Gateway.prototype.nodeTopic = function (node) {
   // clean topic parts
   // eslint-disable-next-line no-redeclare
   for (let i = 0; i < topic.length; i++) {
-    topic[i] = this.mqtt.cleanName(topic[i])
+    topic[i] = utils.sanitizeTopic(topic[i])
   }
 
   return topic.join('/')
@@ -878,9 +872,9 @@ Gateway.prototype.valueTopic = function (node, valueId, returnObject = false) {
 
         topic.push('endpoint_' + (valueId.endpoint || 0))
 
-        topic.push(valueId.propertyName)
+        topic.push(utils.removeSlash(valueId.propertyName))
         if (valueId.propertyKey) {
-          topic.push(valueId.propertyKey)
+          topic.push(utils.removeSlash(valueId.propertyKey))
         }
         break
       case 0: // valueid
@@ -891,9 +885,9 @@ Gateway.prototype.valueTopic = function (node, valueId, returnObject = false) {
         }
         topic.push(valueId.commandClass)
         topic.push(valueId.endpoint || '0')
-        topic.push(valueId.property)
+        topic.push(utils.removeSlash(valueId.property))
         if (valueId.propertyKey) {
-          topic.push(valueId.propertyKey)
+          topic.push(utils.removeSlash(valueId.propertyKey))
         }
         break
     }
@@ -906,7 +900,7 @@ Gateway.prototype.valueTopic = function (node, valueId, returnObject = false) {
 
     // clean topic parts
     for (let i = 0; i < topic.length; i++) {
-      topic[i] = this.mqtt.cleanName(topic[i])
+      topic[i] = utils.sanitizeTopic(topic[i])
     }
 
     const toReturn = {
@@ -1185,7 +1179,9 @@ Gateway.prototype.discoverDevice = function (node, hassDevice) {
         // Set device information using node info
         payload.device = deviceInfo.call(this, node, nodeName)
 
-        hassDevice.object_id = sanitizeId(hassDevice.object_id)
+        hassDevice.object_id = utils
+          .sanitizeTopic(hassDevice.object_id, true)
+          .toLocaleLowerCase()
 
         // Set a friendly name for this component
         payload.name = getEntityName(
@@ -1510,9 +1506,11 @@ Gateway.prototype.discoverValue = function (node, vId) {
         cfg = copy(hassCfg.central_scene)
 
         // Combile unique Object id, by using all possible scenarios
-        cfg.object_id = [cfg.object_id, valueId.propertyKey, valueId.property]
-          .filter(v => !!v)
-          .join('_')
+        cfg.object_id = utils.joinProps(
+          cfg.object_id,
+          valueId.property,
+          valueId.propertyKey
+        )
         break
       case CommandClasses['Binary Sensor']:
         // https://github.com/zwave-js/node-zwave-js/blob/master/packages/zwave-js/src/lib/commandclass/BinarySensorCC.ts#L41
@@ -1523,8 +1521,9 @@ Gateway.prototype.discoverValue = function (node, vId) {
         let sensorTypeName = valueId.property
 
         if (sensorTypeName) {
-          sensorTypeName = this.mqtt.cleanName(
-            sensorTypeName.toLocaleLowerCase()
+          sensorTypeName = utils.sanitizeTopic(
+            sensorTypeName.toLocaleLowerCase(),
+            true
           )
         }
         // TODO: Implement all BinarySensorTypes
@@ -1561,12 +1560,11 @@ Gateway.prototype.discoverValue = function (node, vId) {
         // https://github.com/zwave-js/node-zwave-js/blob/master/packages/zwave-js/src/lib/commandclass/NotificationCC.ts
         // https://github.com/zwave-js/node-zwave-js/blob/master/packages/config/config/notifications.json
         cfg = copy(hassCfg.sensor_generic)
-        if (valueId.propertyKey) {
-          cfg.object_id =
-            'notification_' + valueId.property + '_' + valueId.propertyKey
-        } else {
-          cfg.object_id = 'notification_' + valueId.property
-        }
+        cfg.object_id = utils.joinProps(
+          'notification',
+          valueId.property,
+          valueId.propertyKey
+        )
 
         // TODO: Improve the icons for different propertyKeys!
         switch (valueId.propertyKey) {
@@ -1613,6 +1611,8 @@ Gateway.prototype.discoverValue = function (node, vId) {
               valueId.ccSpecific,
               this.zwave.driver.configManager
             )
+
+            sensor.objectId += '_' + valueId.property
           } else {
             return
           }
@@ -1670,19 +1670,8 @@ Gateway.prototype.discoverValue = function (node, vId) {
         if (isSensor) {
           cfg = copy(hassCfg.sensor_generic)
         }
-        // Assemble an object ID including propertyKey and property.
-        // PropertyKey is set before property as propertyKey can have multiple relevant properties.
-        cfg.object_id =
-          sensor.sensor + (sensor.objectId ? '_' + sensor.objectId : '')
-
-        // If there is a propertyKey, add it on object_id. Without this we cannot generate
-        // all Hass discoveries on sensors with these properties
-        if (valueId.propertyKey) {
-          cfg.object_id +=
-            (valueId.propertyKey ? '_' + valueId.propertyKey : '') +
-            (valueId.property ? '_' + valueId.property : '')
-        }
 
+        cfg.object_id = utils.joinProps(sensor.sensor, sensor.objectId)
         Object.assign(cfg.discovery_payload, sensor.props || {})
 
         // https://github.com/zwave-js/node-zwave-js/blob/master/packages/config/config/scales.json
@@ -1740,15 +1729,16 @@ Gateway.prototype.discoverValue = function (node, vId) {
     payload.device = deviceInfo.call(this, node, nodeName)
 
     // multi instance devices would have same object_id
-    if (valueId.endpoint > 1) cfg.object_id += '_' + valueId.endpoint
+    if (valueId.endpoint) cfg.object_id += '_' + valueId.endpoint
+
+    // remove chars that are not allowed in object ids
+    cfg.object_id = utils.sanitizeTopic(cfg.object_id, true).toLocaleLowerCase()
 
     // Check if another value already exists and add the index to object_id to make it unique
     if (node.hassDevices[cfg.type + '_' + cfg.object_id]) {
       cfg.object_id += '_' + valueId.endpoint
     }
 
-    cfg.object_id = sanitizeId(cfg.object_id)
-
     // Set a friendly name for this component
     payload.name = getEntityName(
       node,
@@ -1759,7 +1749,11 @@ Gateway.prototype.discoverValue = function (node, vId) {
     )
 
     // Set a unique id for the component
-    payload.unique_id = 'zwavejs2mqtt_' + this.zwave.homeHex + '_' + valueId.id
+    payload.unique_id =
+      'zwavejs2mqtt_' +
+      this.zwave.homeHex +
+      '_' +
+      utils.sanitizeTopic(valueId.id, true)
 
     const discoveryTopic = getDiscoveryTopic(cfg, nodeName)
 

diff --git a/lib/MqttClient.js b/lib/MqttClient.js
@@ -49,7 +49,7 @@ function init (config) {
     return
   }
 
-  this.clientID = this.cleanName(NAME_PREFIX + config.name)
+  this.clientID = utils.sanitizeTopic(NAME_PREFIX + config.name)
 
   const parsed = url.parse(config.host || '')
   let protocol = 'mqtt'
@@ -244,13 +244,6 @@ MqttClient.prototype.getClientTopic = function (suffix) {
   return `${this.config.prefix}/${CLIENTS_PREFIX}/${this.clientID}/${suffix}`
 }
 
-MqttClient.prototype.cleanName = function (name) {
-  if (!isNaN(name) || !name) return name
-
-  name = name.replace(/\s/g, '_')
-  return name.replace(/[+*#\\.'`!?^=(),"%[\]:;{}]+/g, '')
-}
-
 /**
  * Method used to close clients connection, use this before destroy
  */

diff --git a/lib/ZwaveClient.js b/lib/ZwaveClient.js
@@ -480,7 +480,7 @@ function onNodeValueAdded (zwaveNode, args) {
  * Emitted when we receive a `value notification` event
  *
  * @param {import('zwave-js').ZWaveNode} zwaveNode
- * @param {import('zwave-js/build/lib/node/Types').ZWaveNodeValueNotificationArgs} args
+ * @param {import('zwave-js').ZWaveNodeValueNotificationArgs} args
  */
 function onNodeValueNotification (zwaveNode, args) {
   // notification hasn't `newValue`
@@ -816,7 +816,7 @@ function initNode (zwaveNode) {
  * Set value metadata to the internal valueId
  *
  * @param {import('zwave-js').ZWaveNode} zwaveNode
- * @param {import('zwave-js').ValueID} zwaveValue
+ * @param {import('zwave-js').TranslatedValueID} zwaveValue
  * @param {import('zwave-js').ValueMetadata} zwaveValueMeta
  * @returns The internal valueId
  */
@@ -832,6 +832,7 @@ function updateValueMetadata (zwaveNode, zwaveValue, zwaveValueMeta) {
     property: zwaveValue.property,
     propertyName: zwaveValue.propertyName,
     propertyKey: zwaveValue.propertyKey,
+    propertyKeyName: zwaveValue.propertyKeyName,
     type: zwaveValueMeta.type, // https://github.com/zwave-js/node-zwave-js/blob/cb35157da5e95f970447a67cbb2792e364b9d1e1/packages/core/src/values/Metadata.ts#L28
     readable: zwaveValueMeta.readable,
     writeable: zwaveValueMeta.writeable,
@@ -876,7 +877,7 @@ function updateValueMetadata (zwaveNode, zwaveValue, zwaveValueMeta) {
  * Add a node value to our node values
  *
  * @param { import('zwave-js').ZWaveNode } zwaveNode
- * @param { import('zwave-js').ValueID } zwaveValue
+ * @param { import('zwave-js').TranslatedValueID } zwaveValue
  */
 function addValue (zwaveNode, zwaveValue) {
   const node = this.nodes[zwaveNode.id]
@@ -900,7 +901,7 @@ function addValue (zwaveNode, zwaveValue) {
  * Parse a zwave value into a valueID
  *
  * @param { import('zwave-js').ZWaveNode } zwaveNode
- * @param { import('zwave-js').ValueID } zwaveValue
+ * @param { import('zwave-js').TranslatedValueID } zwaveValue
  */
 function parseValue (zwaveNode, zwaveValue, zwaveValueMeta) {
   const node = this.nodes[zwaveNode.id]
@@ -1039,7 +1040,7 @@ function isCurrentValue (valueId) {
 /**
  * Find the target valueId of a current valueId
  *
- * @param {import('zwave-js').ValueID} valueId
+ * @param {import('zwave-js').TranslatedValueID} valueId
  * @param {TranslatedValueID[]} definedValueIds
  * @returns
  */
@@ -1049,7 +1050,7 @@ function findTargetValue (zwaveValue, definedValueIds) {
       v.commandClass === zwaveValue.commandClass &&
       v.endpoint === zwaveValue.endpoint &&
       v.propertyKey === zwaveValue.propertyKey &&
-      /target/i.test(v.propertyName)
+      /target/i.test(v.property)
   )
 }
 

diff --git a/lib/utils.js b/lib/utils.js
@@ -16,6 +16,17 @@ module.exports = {
     }
     return path.join(...paths)
   },
+  joinProps (...props) {
+    props = props || []
+    let ret = props[0] || ''
+    for (let i = 1; i < props.length; i++) {
+      const p = props[i]
+      if (p !== null && p !== undefined && p !== '') {
+        ret += '_' + p
+      }
+    }
+    return ret
+  },
   num2hex (num) {
     const hex = num >= 0 ? num.toString(16) : 'XXXX'
     return '0x' + '0'.repeat(4 - hex.length) + hex
@@ -36,6 +47,21 @@ module.exports = {
 
     return VERSION
   },
+  sanitizeTopic (str, removeSlash) {
+    if (!isNaN(str) || !str) return str
+
+    if (removeSlash) {
+      str = this.removeSlash(str)
+    }
+
+    // replace spaces with '_'
+    str = str.replace(/\s/g, '_')
+    // remove special chars
+    return str.replace(/[+*#\\.'`!?^=(),"%[\]:;{}]+/g, '')
+  },
+  removeSlash (str) {
+    return !isNaN(str) ? str : str.replace(/\//g, '-')
+  },
   hasProperty (obj, prop) {
     return Object.prototype.hasOwnProperty.call(obj, prop)
   },

diff --git a/src/components/Settings.vue b/src/components/Settings.vue
@@ -404,11 +404,13 @@
                           (<code>&lt;location-?&gt;&lt;name&gt;</code>)<br />-
                           <code>%nid</code>: Node ID <br />- <code>%n</code>:
                           Node Name <br />- <code>%loc</code>: Node Location
-                          <br />- <code>%pk</code>: valueId property key
-                          (fallback to device type) <br />- <code>%pn</code>:
-                          valueId property name (fallback to device type)
-                          <br />- <code>%o</code>: HASS object_id <br />-
-                          <code>%l</code>: valueId label (fallback to object_id)
+                          <br />- <code>%p</code>: valueId property (fallback to
+                          device type) <br />- <code>%pk</code>: valueId
+                          property key (fallback to device type) <br />-
+                          <code>%pn</code>: valueId property name (fallback to
+                          device type) <br />- <code>%o</code>: HASS object_id
+                          <br />- <code>%l</code>: valueId label (fallback to
+                          object_id)
                         </div>
                       </v-flex>
                     </v-layout>