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.

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

feat(config): support Intermatic PE653 v3.1 with water temperature reading #7545

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/cc/src/cc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1075,3 +1075,4 @@ export {
FibaroVenetianBlindCCReport,
FibaroVenetianBlindCCSet,
} from "./manufacturerProprietary/FibaroCC.js";
export { IntermaticPE653CC } from "./manufacturerProprietary/IntermaticPE653CC.js";
171 changes: 171 additions & 0 deletions packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
import {
CommandClasses,
type GetValueDB,
type MessageOrCCLogEntry,
type MessageRecord,
type ValueID,
ValueMetadata,
type WithAddress,

Check failure on line 8 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'WithAddress' is defined but never used

Check failure on line 8 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'WithAddress' is defined but never used
ZWaveError,
ZWaveErrorCodes,
validatePayload,
} 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 failure on line 15 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'validateArgs' is defined but never used

Check failure on line 15 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'validateArgs' is defined but never used

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import validateArgs.
import {
POLL_VALUE,
type PollValueImplementation,
SET_VALUE,
type SetValueImplementation,
throwUnsupportedProperty,
throwWrongValueType,
} from "../../lib/API.js";
import {
type CCRaw,
type CommandClassOptions,
type InterviewContext,
type PersistValuesContext,
type RefreshValuesContext,
} from "../../lib/CommandClass.js";
import {
ManufacturerProprietaryCC,
ManufacturerProprietaryCCAPI,
} from "../ManufacturerProprietaryCC.js";
import {
manufacturerId,
manufacturerProprietaryAPI,
} from "./Decorators.js";

export const MANUFACTURERID_INTERMATIC_LEGACY = 0x0005;
export const MANUFACTURERID_INTERMATIC_NEW = 0x0072;

/** Returns the ValueID used to store the current water temperature */
export function getIntermaticWaterTempValueId(): ValueID {
return {
commandClass: CommandClasses["Manufacturer Proprietary"],
property: "intermatic",
propertyKey: "waterTemperature",
};
}

/** Returns the value metadata for water temperature */
export function getIntermaticWaterTempMetadata(): ValueMetadata {
return {
...ValueMetadata.Number,
label: "Water Temperature",
unit: "°F",
min: 0,
max: 100,
};
}

@manufacturerProprietaryAPI(MANUFACTURERID_INTERMATIC_LEGACY)
export class IntermaticPE653CCAPI extends ManufacturerProprietaryCCAPI {
public async getWaterTemperature(): Promise<number | undefined> {
const valueId = getIntermaticWaterTempValueId();
return this.getValueDB()?.getValue(valueId);
}

Check failure on line 69 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Async function has no 'await' expression
protected get [SET_VALUE](): SetValueImplementation {
return async function(

Check failure on line 71 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Async function has no 'await' expression
this: IntermaticPE653CCAPI,
{ property },
value,
) {
if (property !== "intermatic") {
throwUnsupportedProperty(this.ccId, property);
}

// This is a read-only CC
throwWrongValueType(this.ccId, property, "readonly", typeof value);
};
}

protected get [POLL_VALUE](): PollValueImplementation {
return async function(this: IntermaticPE653CCAPI, { property }) {
if (property !== "intermatic") {
throwUnsupportedProperty(this.ccId, property);
}

return this.getWaterTemperature();
};
}
}

@manufacturerId(MANUFACTURERID_INTERMATIC_LEGACY)
export class IntermaticPE653CC extends ManufacturerProprietaryCC {
public constructor(options: CommandClassOptions) {
super(options);
this.manufacturerId = MANUFACTURERID_INTERMATIC_LEGACY;
}

public static from(raw: CCRaw, ctx: CCParsingContext): IntermaticPE653CC {
// Check if the manufacturer ID matches either the legacy or new ID
const ccCommand = raw.ccCommand ?? 0;
const manufacturerId = (ccCommand << 8) | raw.payload[0];
if (manufacturerId !== MANUFACTURERID_INTERMATIC_LEGACY &&
manufacturerId !== MANUFACTURERID_INTERMATIC_NEW) {
throw new ZWaveError(
`Invalid manufacturer ID for Intermatic PE653: ${manufacturerId}`,
ZWaveErrorCodes.Argument_Invalid
);
}

const cc = new IntermaticPE653CC({
nodeId: ctx.sourceNodeId,
});
cc.manufacturerId = manufacturerId;

// Validate payload length for PE653 v3.1 water temperature frame
validatePayload(raw.payload.length >= 13, "PE653 frame too short");

// Parse the payload according to the Intermatic PE653 protocol
const waterTemp = raw.payload[12];
cc.waterTemperature = waterTemp;

return cc;
}

public waterTemperature?: number;

Check failure on line 130 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Async method 'interview' has no 'await' expression

public async interview(ctx: InterviewContext): Promise<void> {

Check failure on line 132 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

Async method 'interview' has no 'await' expression
// No interview needed for this CC
this.setInterviewComplete(ctx, true);
}

Check failure on line 135 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'ctx' is defined but never used. Allowed unused args must match /^_/u

public async refreshValues(ctx: RefreshValuesContext): Promise<void> {

Check failure on line 137 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

'ctx' is defined but never used. Allowed unused args must match /^_/u
// This CC is read-only and updates are received automatically
// Skip refresh since this is a read-only CC
}

public persistValues(ctx: PersistValuesContext): boolean {
if (this.waterTemperature != undefined) {
const valueId = getIntermaticWaterTempValueId();
const metadata = getIntermaticWaterTempMetadata();

this.getValueDB(ctx)?.setMetadata(valueId, metadata);
this.getValueDB(ctx)?.setValue(valueId, this.waterTemperature);
return true;
}
return false;
}

public serialize(ctx: CCEncodingContext): Bytes {
// The manufacturer ID is encoded in the first two bytes
const manufacturerId = this.manufacturerId ?? MANUFACTURERID_INTERMATIC_LEGACY;
(this.ccCommand as unknown as number) = (manufacturerId >>> 8) & 0xff;

Check failure on line 157 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

`serialize` is deprecated. Use {@link serializeAsync} instead
this.payload = Bytes.from([manufacturerId & 0xff]);
return super.serialize(ctx);

Check failure on line 159 in packages/cc/src/cc/manufacturerProprietary/IntermaticPE653CC.ts

View workflow job for this annotation

GitHub Actions / lint (18)

`serialize` is deprecated. Use {@link serializeAsync} instead
}

public toLogEntry(ctx?: GetValueDB): MessageOrCCLogEntry {
const message: MessageRecord = {
waterTemperature: this.waterTemperature ?? "unknown",
};
return {
...super.toLogEntry(ctx),
message,
};
}
}
3 changes: 3 additions & 0 deletions packages/cc/src/cc/manufacturerProprietary/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./Decorators.js";
export * from "./FibaroCC.js";
export * from "./IntermaticPE653CC.js";
Copy link
Member

Choose a reason for hiding this comment

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

I think this file should have been copied, not moved. After all there are still users with the old version.

Original file line number Diff line number Diff line change
@@ -1,18 +1,39 @@
{

Check failure on line 1 in packages/config/config/devices/0x0072/pe653.json

View workflow job for this annotation

GitHub Actions / lint-zwave (18)

The properties "endpoints" and "associations" cannot be used together. To define associations for the root endpoint, specify them under endpoint "0"!
"manufacturer": "Intermatic",
"manufacturerId": "0x0005",
"label": "PE653",
"description": "Pool Control",
"manufacturer": "Intermatic 0x0072 WaterTemp",
"manufacturerId": "0x0072",
"label": "PE653 0x0072 WaterTemp",
"description": "Pool Control 0x0072 WaterTemp",
Comment on lines +2 to +5
Copy link
Member

Choose a reason for hiding this comment

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

Just edit the manufacturer ID, keep the rest as-is. Unless the device is marketed under a new name?

"devices": [
{
"productType": "0x5045",
"productId": "0x0653"
"productType": "0x0500",
"productId": "0x7205"
}
],
"firmwareVersion": {
"min": "0.0",
"max": "255.255"
},
"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"
}
}
}
}
},
Comment on lines +16 to +36
Copy link
Member

Choose a reason for hiding this comment

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

This seems unnecessary

"paramInformation": [
{
"#": "1[0x02]",
Expand Down Expand Up @@ -481,72 +502,64 @@
},
"compat": [
{
// 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
Comment on lines -484 to 538
Copy link
Member

Choose a reason for hiding this comment

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

Keep the comments. They explain why each compat flag exists.

},
"Manufacturer Proprietary": {
"isSupported": true,
"version": 1,
"optional": false
}
Comment on lines +540 to 544
Copy link
Member

Choose a reason for hiding this comment

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

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
]
}
]
Comment on lines -537 to -548
Copy link
Member

Choose a reason for hiding this comment

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

Is this issue fixed in this device version?

}
}
]
],
"endpoints": {
"0": {
"commandClasses": {
"0x72": {
"isSupported": true,
"version": 1
},
"0x91": {
"isSupported": true,
"version": 1,
"optional": false
}
}
}
}
Comment on lines +549 to +563
Copy link
Member

Choose a reason for hiding this comment

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

Likewise, I'd like to see the interview log to know if this is necessary.

}

55 changes: 55 additions & 0 deletions packages/intermatic-pe653/src/IntermaticPE653CC.ts
Copy link
Member

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import {
CommandClass,
ManufacturerProprietaryCC,
CCAPI,
ValueMetadata,
ValueID,
} from "zwave-js";
Comment on lines +1 to +7

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import CommandClass.

const INTERMATIC_MANUFACTURER_ID = 0x0005;

export class IntermaticPE653CC extends ManufacturerProprietaryCC {
public static readonly manufacturerId = INTERMATIC_MANUFACTURER_ID;
public static readonly version = 1;

public deserialize(data: Buffer): void {
super.deserialize(data);

if (data.length >= 13) {
const waterTemp = data.readUInt8(12);
this.persistWaterTemp(waterTemp);
}
}

private persistWaterTemp(waterTemp: number): void {
const valueId: ValueID = {
commandClass: this.ccId,
property: "waterTemperature",
};

const metadata: ValueMetadata = {
label: "Water Temperature",
unit: "°F",
valueType: "number",
min: 0,
max: 100,
};

this.getValueDB()?.setMetadata(valueId, metadata);
this.getValueDB()?.setValue(valueId, waterTemp);
}
}

export class IntermaticPE653CCAPI extends CCAPI {
public async getWaterTemperature(): Promise<number | undefined> {
const valueId: ValueID = {
commandClass: this.ccId,
property: "waterTemperature",
};

return this.getValueDB()?.getValue(valueId);
}
}

// Register this CC with Z-Wave JS
ManufacturerProprietaryCC.addImplementation(INTERMATIC_MANUFACTURER_ID, IntermaticPE653CC);
Loading
Loading