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

ZCL types revamp #1033

Merged
merged 9 commits into from
May 1, 2024
Merged

ZCL types revamp #1033

merged 9 commits into from
May 1, 2024

Conversation

Nerivec
Copy link
Collaborator

@Nerivec Nerivec commented Apr 26, 2024

  • Uppercased Zcl DataType (long overdue) and added some comments for good measure
  • Isolated Buffalo and BuffaloZcl so they no longer have traces of zstack-specific logic.
    • Buffalo deals with base types and holds the generic functions.
    • BuffaloZcl adds support for Zcl-specific types.
    • Stack-specific buffalo can add support for stack-specific types as needed.
  • Use enums directly instead of string comparisons for various Buffalo classes.
  • Added missing types read/write to BuffaloZcl with tests.
  • Added tests for missing coverage previously hidden behind "generic" read/write.
  • Added extra checks for options-dependent and "non-value" read/write and tests to go with it.
    • Some write are still missing "non-value" support, impossible because of current implementations. Requires refactoring.
  • Added loads of typing and cleaned up old ones.
  • Added missing Foundation commands with BuffaloZcl support and tests.

TODO:

  • @Koenkk If you can confirm that zstack is happy with the changes. 🤞
  • ZCL types revamp zigbee-herdsman-converters#7456
  • LONG_OCTET_STR was previously using the same read/write as LONG_CHAR_STR, changed to number[]/Buffer. Need to check on the 2 use cases:
    • Clusters.liXeePrivate.attributes.daysProfileCurrentCalendar
    • Clusters.liXeePrivate.attributes.daysProfileNextCalendar
  • Fix inconsistencies with args/return values for some uint/int types (should be plain numbers instead of lsb/msb tuples) then move these basic type read/write from BuffaloZcl to Buffalo. Should probably be in another PR, will affect more than the Zcl definition.

NOTE: zigate missing ParameterType values were added arbitrarily to allow compiling. It seems to also be missing types support. Code looks broken overall.

@Nerivec Nerivec marked this pull request as ready for review April 27, 2024 15:37
@Nerivec
Copy link
Collaborator Author

Nerivec commented Apr 28, 2024

Some quick (i.e. imprecise) performance tests (x1000000):

BuffaloZcl Before After Change
read uint8 ~270ms ~200ms -25%
write uint8 ~190ms ~130ms -31%
read array ~680ms ~350ms -48%
write array ~750ms ~300ms -60%
read enum8 ~290ms ~200ms -31%
write enum8 ~220ms ~130ms -40%
read buffer ~310ms ~310ms -
write buffer ~530ms ~340ms -35%
BuffaloZcl read uint8
for (let x = 0; x < 1000000; x++) {
    const buffalo = new BuffaloZcl(Buffer.from([0x00, 0x03, 0x00, 0x00]), 1);
    buffalo.read(Zcl.DataType.UINT8, {});
    // buffalo.read('uint8', {});
}
BuffaloZcl write uint8
for (let x = 0; x < 1000000; x++) {
    const buffalo = new BuffaloZcl(Buffer.alloc(3), 1);
    buffalo.write(Zcl.DataType.UINT8, 240, {});
    // buffalo.write('uint8', 240, {});
}
BuffaloZcl read array
for (let x = 0; x < 1000000; x++) {
    const buffer = Buffer.from([32, 3, 0, 1, 2, 3]);
    const buffalo = new BuffaloZcl(buffer);
    buffalo.read(Zcl.DataType.ARRAY, {});
    // buffalo.read('array', {});
}
BuffaloZcl write array
for (let x = 0; x < 1000000; x++) {
    const payload = {elementType: Zcl.DataType.DATA8, elements: [0, 0, 0, 0]};
    // const payload = {elementType: Zcl.DataType.data8, elements: [0, 0, 0, 0]};
    const buffer = Buffer.alloc(7);
    const buffalo = new BuffaloZcl(buffer);
    buffalo.write(DataType.ARRAY, payload, {});
    // buffalo.write('array', payload, {});
}
BuffaloZcl read enum8
for (let x = 0; x < 1000000; x++) {
    const buffalo = new BuffaloZcl(Buffer.from([0x00, 0x03, 0x00, 0x00]), 1);
    buffalo.read(Zcl.DataType.ENUM8, {});
    // buffalo.read('enum8', {});
}
BuffaloZcl write enum8
for (let x = 0; x < 1000000; x++) {
    const buffalo = new BuffaloZcl(Buffer.alloc(3), 1);
    buffalo.write(Zcl.DataType.ENUM8, 240, {});
    // buffalo.write('enum8', 240, {});
}
BuffaloZcl read buffer
for (let x = 0; x < 1000000; x++) {
    const buffer = Buffer.from([0x01, 0x02, 0x03, 0x04, 0x05]);
    const buffalo = new BuffaloZcl(buffer);
    buffalo.read(BuffaloZclDataType.BUFFER, {});
    // buffalo.read(BuffaloZclDataType[BuffaloZclDataType.BUFFER], {});
}
BuffaloZcl write buffer
for (let x = 0; x < 1000000; x++) {
    const buffer = Buffer.alloc(5);
    const expected = Buffer.from([0x01, 0x02, 0x03, 0x04, 0x05]);
    const buffalo = new BuffaloZcl(buffer);
    buffalo.write(BuffaloZclDataType.BUFFER, expected, {});
    // buffalo.write(BuffaloZclDataType[BuffaloZclDataType.BUFFER], expected, {});
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants