M-Labs/thermostat
9
0
Fork 5
Code Issues 9 Pull Requests 5 Releases Wiki Activity

Cleanup fan control #89

Merged
sb10q merged 1 commits from atse/thermostat:fan-pwm-cleanup into master 2024-08-17 17:37:18 +08:00
Conversation 4 Commits 1 Files Changed 1 +6 -7
atse commented 2023-08-28 11:26:11 +08:00
Contributor
Copy Link
  • Refactor current_abs_max_tec_i in channels.rs to use uom instead of bare f64s
  • Rework the same function to check for channel current in all channels instead of 2
  • Proofread the fan control section in README.md
- Refactor `current_abs_max_tec_i` in `channels.rs` to use uom instead of bare `f64`s - ~~Rework the same function to check for channel current in all channels instead of 2~~ - Proofread the fan control section in README.md
sb10q commented 2023-11-13 11:22:30 +08:00
Owner
Copy Link

Refactor current_abs_max_tec_i in channels.rs to use uom instead of bare f64s

This cannot be done without changing the structure of the code in current_abs_max_tec_i? Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented.

> Refactor current_abs_max_tec_i in channels.rs to use uom instead of bare f64s This cannot be done without changing the structure of the code in current_abs_max_tec_i? Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented.
atse commented 2023-11-14 17:39:56 +08:00
Author
Contributor
Copy Link

Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented.

Yes, this is exactly what I had done initially in that commit. I only changed the structure to generalize the function to deal with more than two channels, since max_by only compares two values.

> Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented. Yes, this is exactly what I had done initially in that commit. I only changed the structure to generalize the function to deal with more than two channels, since max_by only compares two values.
sb10q reviewed 2024-01-26 17:02:46 +08:00
README.md Outdated
@ -279,3 +279,3 @@
## Fan control
Fan control is available for the thermostat revisions with integrated fan system. For this purpose four commands are available:
Fan control is available for thermostat revisions with an integrated fan system. Five commands are available for this purpose:
sb10q commented 2024-01-26 17:02:46 +08:00
Owner
Copy Link

I don't think anyone cares about the number of commands and it's hassle to maintain (as this commit shows). Remove that number.

I don't think anyone cares about the number of commands and it's hassle to maintain (as this commit shows). Remove that number.
👍 1
atse commented 2024-01-26 17:28:29 +08:00
Author
Contributor
Copy Link

Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented.

Yes, this is exactly what I had done initially in that commit. I only changed the structure to generalize the function to deal with more than two channels, since max_by only compares two values.

Also thinking about this now, I don't think there is a need to generalize for more than 2 channels anyway right? I believe the EEM version already has its own fan PWM implementation.

> > Usually those Rust functions like max_by have type parameters and will work if used correctly and the right traits are implemented. > > Yes, this is exactly what I had done initially in that commit. I only changed the structure to generalize the function to deal with more than two channels, since max_by only compares two values. Also thinking about this now, I don't think there is a need to generalize for more than 2 channels anyway right? I believe the EEM version already has its own fan PWM implementation.
sb10q commented 2024-01-26 17:30:11 +08:00
Owner
Copy Link

The EEM version is MQTT based. I was hoping this could be the non-MQTT version (but could still use miniconf) with custom GUI, for both Thermostat devices.

The EEM version is MQTT based. I was hoping this could be the non-MQTT version (but could still use miniconf) with custom GUI, for both Thermostat devices.
👍 1
atse force-pushed fan-pwm-cleanup from 59e0365669 to 17edae44fb 2024-01-30 12:44:05 +08:00 Compare
sb10q merged commit 17edae44fb into master 2024-01-30 14:14:22 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/thermostat#89
No description provided.
Delete Branch "atse/thermostat:fan-pwm-cleanup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?