Support fan PWM settings #73

Merged
sb10q merged 16 commits from esavkin/thermostat:69-fan_pwm into master 2023-03-22 17:15:49 +08:00
5 changed files with 54 additions and 14 deletions
Showing only changes of commit 66143d2373 - Show all commits

View File

@ -125,7 +125,7 @@ formatted as line-delimited JSON.
| `dfu` | Reset device and enters USB device firmware update (DFU) mode | | `dfu` | Reset device and enters USB device firmware update (DFU) mode |
| `ipv4 <X.X.X.X/L> [Y.Y.Y.Y]` | Configure IPv4 address, netmask length, and optional default gateway | | `ipv4 <X.X.X.X/L> [Y.Y.Y.Y]` | Configure IPv4 address, netmask length, and optional default gateway |
| `fan` | Show current fan settings and sensors' measurements | | `fan` | Show current fan settings and sensors' measurements |
| `fan <value>` | Set fan power with values from 0 to 100, where 0 is auto mode (TODO) | | `fan <value>` | Set fan power with values from 0 to 100, where 0 is auto mode |
## USB ## USB
@ -271,3 +271,13 @@ with the following keys.
## PID Tuning ## PID Tuning
The thermostat implements a PID control loop for each of the TEC channels, more details on setting up the PID control loop can be found [here](./doc/PID%20tuning.md). The thermostat implements a PID control loop for each of the TEC channels, more details on setting up the PID control loop can be found [here](./doc/PID%20tuning.md).
## Fan control
Fan control is available for the thermostat revisions with integrated fan system. For this purpose two commands are available:
1. `fan` - show fan stats: `fan_pwm`, `tacho`, `abs_max_tec_i`, `auto_mode`. Please note that `tacho` shows *approximate* value, which
sb10q marked this conversation as resolved
Review

Why is it so approximate?

Why is it so approximate?
Review

Mostly because I cannot guarantee it is precise and would not like to see how someone would tie something-very-important to its value.

Mostly because I cannot guarantee it is precise and would not like to see how someone would tie something-very-important to its value.
linearly correlates with the actual fan speed.
2. `fan <value>` - set the fan power with the value from `0` to `100`. Since there is no hardware way to disable the fan,
`0` value is used for enabling automatic fan control mode, which correlates with the square of the TEC's current.
Values from `1` to `100` are used for setting the power from minimum to maximum respectively.
Outdated
Review

Doesn't the motor stall at low powers?

Doesn't the motor stall at low powers?

No, the minimum I tried was 0.008 PWM, anything below that makes the circuit consider there is no signal and it runs at full power. 0.008-0.04 values are working quite fine, though there is specific noise on low speed.

No, the minimum I tried was 0.008 PWM, anything below that makes the circuit consider there is no signal and it runs at full power. 0.008-0.04 values are working quite fine, though there is specific noise on low speed.
Outdated
Review

"correlates with the square of the TEC's current" is incorrect

"correlates with the square of the TEC's current" is incorrect
Please note that power doesn't correlate with the actual speed linearly.

View File

@ -26,6 +26,9 @@ pub const CHANNELS: usize = 2;
pub const R_SENSE: f64 = 0.05; pub const R_SENSE: f64 = 0.05;
// DAC chip outputs 0-5v, which is then passed through a resistor dividor to provide 0-3v range // DAC chip outputs 0-5v, which is then passed through a resistor dividor to provide 0-3v range
const DAC_OUT_V_MAX: f64 = 3.0; const DAC_OUT_V_MAX: f64 = 3.0;
const MAX_TEC_I: f64 = 3.0; // as stated in the schemes
esavkin marked this conversation as resolved Outdated
Outdated
Review

schematics

schematics
const MAX_FAN_PWM: f64 = 100.0;
const MIN_FAN_PWM: f64 = 1.0;
#[derive(Serialize, Copy, Clone)] #[derive(Serialize, Copy, Clone)]
pub struct HWRev { pub struct HWRev {
@ -41,7 +44,8 @@ pub struct Channels {
/// stm32f4 integrated adc /// stm32f4 integrated adc
pins_adc: pins::PinsAdc, pins_adc: pins::PinsAdc,
pub pwm: pins::PwmPins, pub pwm: pins::PwmPins,
pub hw_rev: HWRev hw_rev: HWRev,
fan_auto: bool
} }
impl Channels { impl Channels {
@ -63,7 +67,8 @@ impl Channels {
let channel1 = Channel::new(pins.channel1, adc_calibration1); let channel1 = Channel::new(pins.channel1, adc_calibration1);
let pins_adc = pins.pins_adc; let pins_adc = pins.pins_adc;
let pwm = pins.pwm; let pwm = pins.pwm;
let mut channels = Channels { channel0, channel1, adc, pins_adc, pwm, hw_rev: Self::detect_hw_rev(&pins.hwrev) }; let mut channels = Channels { channel0, channel1, adc, pins_adc, pwm,
hw_rev: Self::detect_hw_rev(&pins.hwrev), fan_auto: true };
for channel in 0..CHANNELS { for channel in 0..CHANNELS {
channels.channel_state(channel).vref = channels.read_vref(channel); channels.channel_state(channel).vref = channels.read_vref(channel);
channels.calibrate_dac_value(channel); channels.calibrate_dac_value(channel);
@ -407,7 +412,7 @@ impl Channels {
(self.read_tec_u_meas(channel) - ElectricPotential::new::<volt>(1.5)) * 4.0 (self.read_tec_u_meas(channel) - ElectricPotential::new::<volt>(1.5)) * 4.0
} }
pub fn set_pwm(&mut self, channel: usize, pin: PwmPin, duty: f64) -> f64 { fn set_pwm(&mut self, channel: usize, pin: PwmPin, duty: f64) -> f64 {
fn set<P: hal::PwmPin<Duty=u16>>(pin: &mut P, duty: f64) -> f64 { fn set<P: hal::PwmPin<Duty=u16>>(pin: &mut P, duty: f64) -> f64 {
let max = pin.get_max_duty(); let max = pin.get_max_duty();
let value = ((duty * (max as f64)) as u16).min(max); let value = ((duty * (max as f64)) as u16).min(max);
@ -458,12 +463,16 @@ impl Channels {
} }
pub fn set_fan_pwm(&mut self, fan_pwm: u32) -> f64 { pub fn set_fan_pwm(&mut self, fan_pwm: u32) -> f64 {
let duty = fan_pwm as f64 / 100.0; let duty = fan_pwm as f64 / MAX_FAN_PWM;
self.set_pwm(0, PwmPin::Fan, duty) self.set_pwm(0, PwmPin::Fan, duty)
} }
pub fn get_fan_pwm(&mut self) -> u32 { pub fn get_fan_pwm(&mut self) -> u32 {
(self.get_pwm(0, PwmPin::Fan) * 100.0) as u32 (self.get_pwm(0, PwmPin::Fan) * MAX_FAN_PWM) as u32
}
pub fn set_fan_auto_mode(&mut self, fan_auto: bool) {
self.fan_auto = fan_auto;
} }
fn report(&mut self, channel: usize) -> Report { fn report(&mut self, channel: usize) -> Report {
@ -557,12 +566,19 @@ impl Channels {
serde_json_core::to_vec(&summaries) serde_json_core::to_vec(&summaries)
} }
fn current_abs_max_tec_i(&mut self) -> f64 {
max_by(self.get_tec_i(0).abs().get::<ampere>(),
self.get_tec_i(1).abs().get::<ampere>(),
|a, b| a.partial_cmp(b).unwrap_or(core::cmp::Ordering::Equal))
}
pub fn fan_summary(&mut self, tacho: Option<u32>) -> Result<JsonBuffer, serde_json_core::ser::Error> { pub fn fan_summary(&mut self, tacho: Option<u32>) -> Result<JsonBuffer, serde_json_core::ser::Error> {
if self.fan_available() { if self.fan_available() {
let summary = FanSummary { let summary = FanSummary {
fan_pwm: self.get_fan_pwm(), fan_pwm: self.get_fan_pwm(),
tacho: tacho.unwrap_or(u32::MAX), tacho: tacho.unwrap_or(u32::MAX),
abs_max_tec_i: max_by(self.get_tec_i(0).abs().value, self.get_tec_i(1).abs().value, |a, b| a.partial_cmp(b).unwrap()) abs_max_tec_i: self.current_abs_max_tec_i(),
auto_mode: self.fan_auto
}; };
serde_json_core::to_vec(&summary) serde_json_core::to_vec(&summary)
} else { } else {
@ -570,6 +586,16 @@ impl Channels {
serde_json_core::to_vec(&summary) serde_json_core::to_vec(&summary)
} }
} }
pub fn fan_ctrl(&mut self) {
if self.fan_auto && self.fan_available() {
let scaled_current = self.current_abs_max_tec_i() / MAX_TEC_I;
Outdated
Review

Noise can potentially make this >1 with maximum TEC current settings. What happens then?

Noise can potentially make this >1 with maximum TEC current settings. What happens then?

It would still run on 1.0, as there is a limiter in the PWM setter.

It would still run on 1.0, as there is a limiter in the PWM setter.
let pwm = max_by(scaled_current * scaled_current * MAX_FAN_PWM,
Outdated
Review

You should make the slope configurable and then experiment on the hardware to find a reasonable default.

You should make the slope configurable and then experiment on the hardware to find a reasonable default.

Introduce a,b,c of quadratic equation coefficients?

Introduce a,b,c of quadratic equation coefficients?
Outdated
Review

Just a*current**2 + b should be enough...

Just ``a*current**2 + b`` should be enough...

c would add ability to make it nearly linear

`c` would add ability to make it nearly linear

Also, that may be useful for GUI, as user could choose any 3 points on the fan graph.

Also, that may be useful for GUI, as user could choose any 3 points on the fan graph.
Outdated
Review

I doubt we need a GUI for this, the hardware is fixed.

I doubt we need a GUI for this, the hardware is fixed.
MIN_FAN_PWM,
|a, b| a.partial_cmp(b).unwrap_or(core::cmp::Ordering::Equal)) as u32;
self.set_fan_pwm(pwm);
Outdated
Review

f64 really? Now we're talking high precision fan control!

f64 really? Now we're talking high precision fan control!

I think we can go with int range from 1 to 100, since fan seems to not support values less than 0.01

I think we can go with int range from 1 to 100, since fan seems to not support values less than 0.01
}
}
} }
type JsonBuffer = Vec<u8, U1024>; type JsonBuffer = Vec<u8, U1024>;
@ -650,4 +676,5 @@ pub struct FanSummary {
fan_pwm: u32, fan_pwm: u32,
tacho: u32, tacho: u32,
abs_max_tec_i: f64, abs_max_tec_i: f64,
auto_mode: bool,
} }

View File

@ -347,13 +347,14 @@ impl Handler {
fn fan (socket: &mut TcpSocket, channels: &mut Channels, fan_pwm: Option<u32>, tacho_value: Option<u32>) -> Result<Handler, Error> { fn fan (socket: &mut TcpSocket, channels: &mut Channels, fan_pwm: Option<u32>, tacho_value: Option<u32>) -> Result<Handler, Error> {
match fan_pwm { match fan_pwm {
Outdated
Review

This behavior is inconsistent with the pwm command, the latter uses pwm i_set <current> / pwm pid, not pwm i_set 0 to enable PID.

This behavior is inconsistent with the ``pwm`` command, the latter uses ``pwm i_set <current> / pwm pid``, not ``pwm i_set 0`` to enable PID.
Some(val) => { Some(val) => {
channels.set_fan_auto_mode(val == 0);
channels.set_fan_pwm(val); channels.set_fan_pwm(val);
Ok(Handler::Handled)
}, },
None => { None => {
match channels.fan_summary(tacho_value) { match channels.fan_summary(tacho_value) {
Ok(buf) => { Ok(buf) => {
Outdated
Review

Why does this get printed on !is_default_auto()? Looks like function naming or layout could still use some work....

Why does this get printed on ``!is_default_auto()``? Looks like function naming or layout could still use some work....
send_line(socket, &buf); send_line(socket, &buf);
return Ok(Handler::Handled);
} }
Err(e) => { Err(e) => {
Outdated
Review

at your own risk

at your own risk
error!("unable to serialize fan summary: {:?}", e); error!("unable to serialize fan summary: {:?}", e);
@ -361,9 +362,10 @@ impl Handler {
return Err(Error::ReportError); return Err(Error::ReportError);
} }
}; };
Ok(Handler::Handled)
} }
} };
send_line(socket, b"{}");
Ok(Handler::Handled)
} }
Outdated
Review

Remove space before ( (also in the other functions)

Remove space before ( (also in the other functions)
pub fn handle_command (command: Command, socket: &mut TcpSocket, channels: &mut Channels, session: &Session, leds: &mut Leds, store: &mut FlashStore, ipv4_config: &mut Ipv4Config, tacho_value: Option<u32>) -> Result<Self, Error> { pub fn handle_command (command: Command, socket: &mut TcpSocket, channels: &mut Channels, session: &Session, leds: &mut Leds, store: &mut FlashStore, ipv4_config: &mut Ipv4Config, tacho_value: Option<u32>) -> Result<Self, Error> {

View File

@ -303,8 +303,7 @@ fn pwm_setup(input: &[u8]) -> IResult<&[u8], Result<(PwmPin, f64), Error>> {
) )
), ),
result_with_pin(PwmPin::MaxV) result_with_pin(PwmPin::MaxV)
), ))
)
)(input) )(input)
} }

View File

@ -19,14 +19,14 @@ use stm32f4xx_hal::{
stm32::{CorePeripherals, Peripherals, SCB}, stm32::{CorePeripherals, Peripherals, SCB},
time::{U32Ext, MegaHertz}, time::{U32Ext, MegaHertz},
watchdog::IndependentWatchdog, watchdog::IndependentWatchdog,
gpio::{Edge, ExtiPin},
syscfg::SysCfgExt
}; };
use smoltcp::{ use smoltcp::{
time::Instant, time::Instant,
socket::TcpSocket, socket::TcpSocket,
wire::EthernetAddress, wire::EthernetAddress,
}; };
use stm32f4xx_hal::gpio::{Edge, ExtiPin};
use stm32f4xx_hal::syscfg::SysCfgExt;
mod init_log; mod init_log;
use init_log::init_log; use init_log::init_log;
@ -220,6 +220,8 @@ fn main() -> ! {
} }
} }
channels.fan_ctrl();
cortex_m::interrupt::free(net::clear_pending); cortex_m::interrupt::free(net::clear_pending);
server.poll(instant) server.poll(instant)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {