feat(config): support Intermatic PE653 v3.1 with water temperature reading #7545
Conversation
function1st
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } from "@zwave-js/core/safe"; | ||
| import { type CCEncodingContext, type CCParsingContext } from "@zwave-js/cc"; | ||
| import { Bytes } from "@zwave-js/shared"; | ||
| import { validateArgs } from "@zwave-js/transformers"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
| import { | ||
| CommandClass, | ||
| ManufacturerProprietaryCC, | ||
| CCAPI, | ||
| ValueMetadata, | ||
| ValueID, | ||
| } from "zwave-js"; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
There was a problem hiding this comment.
Can you explain what this file has to do with your changes?
There was a problem hiding this comment.
These implementations cannot be here. If they are needed, they must be in the packages/cc/src/cc/manufacturerProprietary folder where the other one is located too. You can call the one using the legacy manufacturer ID IntermaticPE653LegacyCC or so to avoid naming collisions.
However it does look like this code isn't actually used? As is, it wouldn't build.
There was a problem hiding this comment.
I think this file should have been copied, not moved. After all there are still users with the old version.
| "manufacturer": "Intermatic 0x0072 WaterTemp", | ||
| "manufacturerId": "0x0072", | ||
| "label": "PE653 0x0072 WaterTemp", | ||
| "description": "Pool Control 0x0072 WaterTemp", |
There was a problem hiding this comment.
Just edit the manufacturer ID, keep the rest as-is. Unless the device is marketed under a new name?
| "proprietary": { | ||
| "manufacturerProprietaryConfig": { | ||
| "manufacturerId": 114, | ||
| "commandClasses": { | ||
| "waterTemperature": { | ||
| "type": "number", | ||
| "unit": "°F", | ||
| "min": 0, | ||
| "max": 100, | ||
| "readOnly": true, | ||
| "valueChangeOptions": ["always"], | ||
| "stateless": false, | ||
| "isFromConfig": true, | ||
| "ccSpecific": { | ||
| "propertyName": "waterTemperature", | ||
| "propertyKey": "waterTemperature" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
| // Fixes #4588: Firmware v3.4 has numerous bugs related to multi-endpoint support. | ||
| // Firmware v3.3 and v3.1 do not appear to have the same issues. | ||
| "$if": "firmwareVersion === 3.4", | ||
| "commandClasses": { | ||
| // Force use of Multi Channel CC V1 despite the device reporting V2 | ||
| "add": { | ||
| "Multi Channel": { | ||
| "isSupported": true, | ||
| "version": 1 | ||
| } | ||
| }, | ||
| // The firmware handles requests on some endpoints incorrectly, often reporting garbage | ||
| // that confuses discovery or inhibits operation. Remove all of these broken CCs. | ||
| "remove": { | ||
| // BasicCC: All endpoints control the state of Switch 1 so only keep the root endpoint | ||
| // to reduce clutter and to handle received BASIC_SET events. | ||
| "Basic": { | ||
| "endpoints": [1, 2, 3, 4, 5] | ||
| }, | ||
| // ManufacturerSpecificCC: Endpoint 1 erroneously reports an incorrect manufacturer | ||
| // and product ID, unlike on the root endpoint. | ||
| "Manufacturer Specific": { | ||
| "endpoints": [1] | ||
| }, | ||
| // ClockCC: Endpoint 1 erroneously reports a time with an invalid minute field, | ||
| // unlike on the root endpoint. | ||
| "Clock": { | ||
| "endpoints": [1] | ||
| }, | ||
| // AssociationCC: Endpoint 1 erroneously reports that it supports 133 associated nodes | ||
| // but association commands don't work at all, unlike on the root endpoint. | ||
| "Association": { | ||
| "endpoints": [1] | ||
| }, | ||
| // VersionCC: Endpoint 1 reports an unknown version, unlike on the root endpoint. | ||
| "Version": { | ||
| "endpoints": [1] | ||
| } | ||
| } | ||
| }, | ||
| // The device sometimes sends BASIC_SET to the lifeline association when the state of Switch 1 | ||
| // changes but the value is always 0 so treat it as an event. | ||
| "mapBasicSet": "event" | ||
| }, | ||
| { | ||
| "commandClasses": { | ||
| // Force use of Multi Channel CC V1 despite the device reporting V2 | ||
| "add": { | ||
| "Multi Channel": { | ||
| "isSupported": true, | ||
| "version": 1 |
There was a problem hiding this comment.
Keep the comments. They explain why each compat flag exists.
| "Manufacturer Proprietary": { | ||
| "isSupported": true, | ||
| "version": 1, | ||
| "optional": false | ||
| } |
There was a problem hiding this comment.
Can you share an interview log? I'd expect this to be advertised as supported.
| }, | ||
| "overrideQueries": { | ||
| // The response to the setpoint query is off by one bit: https://github.com/zwave-js/node-zwave-js/issues/5335 | ||
| "Thermostat Setpoint": [ | ||
| { | ||
| "method": "getSupportedSetpointTypes", | ||
| "result": [ | ||
| 1, // Heating | ||
| 7 // Furnace | ||
| ] | ||
| } | ||
| ] |
There was a problem hiding this comment.
Is this issue fixed in this device version?
| "endpoints": { | ||
| "0": { | ||
| "commandClasses": { | ||
| "0x72": { | ||
| "isSupported": true, | ||
| "version": 1 | ||
| }, | ||
| "0x91": { | ||
| "isSupported": true, | ||
| "version": 1, | ||
| "optional": false | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Likewise, I'd like to see the interview log to know if this is necessary.
|
Additionally, do you have some documentation for that proprietary protocol? I wonder what else is in there. |
Pull Request
Title
feat(config): support Intermatic PE653 v3.1 with water temperature reading
Description
This pull request adds support for the Intermatic PE653 device running firmware v3.1, tested using a Raspberry Pi 5 and a RaZberry 7 Z-Wave serial device. The patch updates both the config file and the command class code so that the water temperature reading is parsed correctly. In current testing, the UI label may display this measurement as "Air Temperature," but it is effectively the pool water temperature for the PE653.
Changes
Testing
• Tested the modified config and command class on a device running firmware v3.1.
• Confirmed that the water temperature is read and displayed (labeled as “Air Temperature” in the UI, but functionally correct).
• Verified no regression occurs with legacy firmware or other devices.