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

fix: Move ZclFrame.fromBuffer() out of adapter code #1011

Merged
merged 18 commits into from
Apr 15, 2024

Conversation

Koenkk
Copy link
Owner

@Koenkk Koenkk commented Apr 6, 2024

Move the ZclFrame.fromBuffer() out of adapter code to prepare for custom cluster definitions as discussed in #971

src/adapter/events.ts Outdated Show resolved Hide resolved
src/zcl/zclHeader.ts Outdated Show resolved Hide resolved
@Nerivec
Copy link
Collaborator

Nerivec commented Apr 8, 2024

A few more suggestions...

  • Moving this function to ZclHeader as write() (or something like that) for consistency. Also, frameControl doesn't need a variable in there, it can just be passed directly to buffalo.writeUInt8.

  • Have something like this in ZclHeader:

    /** Returns the amount of bytes used by this header */
    get length(): number {
        return 3 + (this.manufacturerCode === null ? 0 : 2);
    }

    get isGlobal(): boolean {
        return this.frameControl.frameType === FrameType.GLOBAL;
    }

    get isSpecific(): boolean {
        return this.frameControl.frameType === FrameType.SPECIFIC;
    }
  • In ZclFrame, this.Header.commandIdentifier is a duplicate of this.Command.ID. Also, frame.getCommand() seems to be called often enough just to get the ID (frame.getCommand().ID). Probably some optimizations needed around this.

@Koenkk Koenkk marked this pull request as ready for review April 11, 2024 20:58
@Koenkk
Copy link
Owner Author

Koenkk commented Apr 13, 2024

All done! Let me know if this is OK now.

@Koenkk Koenkk requested a review from Nerivec April 13, 2024 20:47
@Koenkk Koenkk mentioned this pull request Apr 14, 2024
Copy link
Collaborator

@Nerivec Nerivec left a comment

Choose a reason for hiding this comment

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

Looks good!

@Koenkk Koenkk merged commit d4796de into master Apr 15, 2024
2 checks passed
@Koenkk Koenkk deleted the fix/from-buffer-refactor branch April 15, 2024 19:44
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.

2 participants