M-Labs
/
thermostat
9
0
Fork 4
Code Issues 8 Pull Requests 7 Releases Wiki Activity

Cleanup fan control #89

Merged
sb10q merged 2 commits from atse/thermostat:fan-pwm-cleanup into master 2024-01-30 14:14:22 +08:00
Conversation 4 Commits 2 Files Changed 4
atse commented 2023-08-28 11:26:11 +08:00
  • 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
atse added 3 commits 2023-08-28 11:26:12 +08:00
e344c11562 current_abs_max_tec_i from all channels, not 0 & 1 
Helps if a thermostat was made with more than 2 channels.
sb10q commented 2023-11-13 11:22:30 +08:00
Owner

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
Poster

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

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
Poster

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

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
There is no content yet.
Delete Branch "atse/thermostat:fan-pwm-cleanup"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?