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: speed-controlled fans #24483

Merged
merged 1 commit into from
Feb 21, 2025
Merged

feat: speed-controlled fans #24483

merged 1 commit into from
Feb 21, 2025

Conversation

lorenz
Copy link
Contributor

@lorenz lorenz commented Oct 25, 2024

This implements the discovery side for speed-controlled fans. Here we no
longer need to emulate speed changes by changing modes and there is a
readily-usable onOff cluster for state. Fans now need to be either mode-
or speed-controlled, otherwise an assert is hit. No such fans currently
exist in the Z2M codebase and they cannot be reasonably controlled
anyways.

Co-dependent with Koenkk/zigbee-herdsman-converters#8188

Copy link
Contributor

This pull request is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale issues label Feb 12, 2025
@lorenz
Copy link
Contributor Author

lorenz commented Feb 15, 2025

Not stale, depends on another PR.

@github-actions github-actions bot removed the stale Stale issues label Feb 16, 2025
@Koenkk
Copy link
Owner

Koenkk commented Feb 20, 2025

@lorenz could you take a look at the failing CI?

@lorenz lorenz force-pushed the fan-speed branch 5 times, most recently from 78c8f50 to 4e8ff68 Compare February 20, 2025 18:51
@lorenz
Copy link
Contributor Author

lorenz commented Feb 20, 2025

Most things are fixed, now it wants me to add test coverage for an assert statement, I'm not sure how I would even test that considering that an assert basically aborts the NodeJS runtime.

@Nerivec
Copy link
Collaborator

Nerivec commented Feb 20, 2025

You can find plenty of examples in current tests, usually in that format:

expect(() => {
   // TODO
}).toThrow('The error message');

This implements the discovery side for speed-controlled fans. Here we no
longer need to emulate speed changes by changing modes and there is a
readily-usable onOff cluster for state. Fans now need to be either mode-
or speed-controlled, otherwise an assert is hit. No such fans currently
exist in the Z2M codebase and they cannot be reasonably controlled
anyways.
@lorenz
Copy link
Contributor Author

lorenz commented Feb 20, 2025

Fixed it by removing the branch and using the assertion value.

@Koenkk Koenkk merged commit 7ce5b58 into Koenkk:dev Feb 21, 2025
13 checks passed
@Koenkk
Copy link
Owner

Koenkk commented Feb 21, 2025

Thanks!

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