Fix PR comments

Signed-off-by: Egor Savkin <es@m-labs.hk>

README.md

@@ -279,7 +279,7 @@ The thermostat implements a PID control loop for each of the TEC channels, more
 
 Fan control is available for the thermostat revisions with integrated fan system. For this purpose four commands are available:
 1. `fan` - show fan stats: `fan_pwm`, `abs_max_tec_i`, `auto_mode`, `k_a`, `k_b`, `k_c`.
-2. `fan auto` - enable auto speed controller mode, which correlates with the square of the TEC's current.
+2. `fan auto` - enable auto speed controller mode, which correlates with fan curve `fcurve`.
 3. `fan <value>` - set the fan power with the value from `1` to `100` and disable auto mode. There is no way to disable the fan.
 Please note that power doesn't correlate with the actual speed linearly.
 4. `fcurve <a> <b> <c>` - set coefficients of the controlling curve `a*x^2 + b*x + c`, where `x` is `abs_max_tec_i/MAX_TEC_I`, 

src/command_handler.rs

@@ -381,29 +381,29 @@ impl Handler {
         Ok(Handler::Handled)
     }
 
-    pub fn handle_command(command: Command, socket: &mut TcpSocket, session: &Session, leds: &mut Leds, store: &mut FlashStore, ipv4_config: &mut Ipv4Config, fan_ctrl: &mut FanCtrl) -> 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, fan_ctrl: &mut FanCtrl) -> Result<Self, Error> {
         match command {
             Command::Quit => Ok(Handler::CloseSocket),
             Command::Reporting(_reporting) => Handler::reporting(socket),            
             Command::Show(ShowCommand::Reporting) => Handler::show_report_mode(socket, session),            
-            Command::Show(ShowCommand::Input) => Handler::show_report(socket, &mut fan_ctrl.channels),
+            Command::Show(ShowCommand::Input) => Handler::show_report(socket, channels),
-            Command::Show(ShowCommand::Pid) => Handler::show_pid(socket, &mut fan_ctrl.channels),
+            Command::Show(ShowCommand::Pid) => Handler::show_pid(socket, channels),
-            Command::Show(ShowCommand::Pwm) => Handler::show_pwm(socket, &mut fan_ctrl.channels),
+            Command::Show(ShowCommand::Pwm) => Handler::show_pwm(socket, channels),
-            Command::Show(ShowCommand::SteinhartHart) => Handler::show_steinhart_hart(socket, &mut fan_ctrl.channels),
+            Command::Show(ShowCommand::SteinhartHart) => Handler::show_steinhart_hart(socket, channels),
-            Command::Show(ShowCommand::PostFilter) => Handler::show_post_filter(socket, &mut fan_ctrl.channels),
+            Command::Show(ShowCommand::PostFilter) => Handler::show_post_filter(socket, channels),
             Command::Show(ShowCommand::Ipv4) => Handler::show_ipv4(socket, ipv4_config),
-            Command::PwmPid { channel } => Handler::engage_pid(socket, &mut fan_ctrl.channels, leds, channel),
+            Command::PwmPid { channel } => Handler::engage_pid(socket, channels, leds, channel),
-            Command::Pwm { channel, pin, value } => Handler::set_pwm(socket, &mut fan_ctrl.channels, leds, channel, pin, value),
+            Command::Pwm { channel, pin, value } => Handler::set_pwm(socket, channels, leds, channel, pin, value),
-            Command::CenterPoint { channel, center } => Handler::set_center_point(socket, &mut fan_ctrl.channels, channel, center),
+            Command::CenterPoint { channel, center } => Handler::set_center_point(socket, channels, channel, center),
-            Command::Pid { channel, parameter, value } => Handler::set_pid(socket, &mut fan_ctrl.channels, channel, parameter, value),
+            Command::Pid { channel, parameter, value } => Handler::set_pid(socket, channels, channel, parameter, value),
-            Command::SteinhartHart { channel, parameter, value } => Handler::set_steinhart_hart(socket, &mut fan_ctrl.channels, channel, parameter, value),
+            Command::SteinhartHart { channel, parameter, value } => Handler::set_steinhart_hart(socket, channels, channel, parameter, value),
-            Command::PostFilter { channel, rate: None } => Handler::reset_post_filter(socket, &mut fan_ctrl.channels, channel),
+            Command::PostFilter { channel, rate: None } => Handler::reset_post_filter(socket, channels, channel),
-            Command::PostFilter { channel, rate: Some(rate) } => Handler::set_post_filter(socket, &mut fan_ctrl.channels, channel, rate),
+            Command::PostFilter { channel, rate: Some(rate) } => Handler::set_post_filter(socket, channels, channel, rate),
-            Command::Load { channel } => Handler::load_channel(socket, &mut fan_ctrl.channels, store, channel),
+            Command::Load { channel } => Handler::load_channel(socket, channels, store, channel),
-            Command::Save { channel } => Handler::save_channel(socket, &mut fan_ctrl.channels, channel, store),
+            Command::Save { channel } => Handler::save_channel(socket, channels, channel, store),
             Command::Ipv4(config) => Handler::set_ipv4(socket, store, config),
-            Command::Reset => Handler::reset(&mut fan_ctrl.channels),
+            Command::Reset => Handler::reset(channels),
-            Command::Dfu => Handler::dfu(&mut fan_ctrl.channels),
+            Command::Dfu => Handler::dfu(channels),
             Command::FanSet {fan_pwm} => Handler::set_fan(socket, fan_pwm, fan_ctrl),
             Command::ShowFan => Handler::show_fan(socket, fan_ctrl),
             Command::FanAuto => Handler::fan_auto(socket, fan_ctrl),

src/fan_ctrl.rs

@@ -1,3 +1,4 @@
+use num_traits::Float;
 use serde::Serialize;
 use stm32f4xx_hal::{
     pwm::{self, PwmChannels},
@@ -6,7 +7,7 @@ use stm32f4xx_hal::{
 
 use crate::{
     pins::HWRevPins,
-    channels::{Channels, JsonBuffer},
+    channels::JsonBuffer,
 };
 
 pub type FanPin = PwmChannels<TIM8, pwm::C4>;
@@ -17,8 +18,8 @@ const MAX_TEC_I: f64 = 3.0;
 const MAX_USER_FAN_PWM: f64 = 100.0;
 const MIN_USER_FAN_PWM: f64 = 1.0;
 const MAX_FAN_PWM: f64 = 1.0;
-// below this value, motor pulse signal is too weak
+// below this value motor circuit assumes there is no signal and runs full speed
-const MIN_FAN_PWM: f64 = 0.05;
+const MIN_FAN_PWM: f64 = 0.02;
 
 const DEFAULT_K_A: f64 = 1.0;
 const DEFAULT_K_B: f64 = 0.0;
@@ -38,12 +39,12 @@ pub struct FanCtrl {
     k_a: f64,
     k_b: f64,
     k_c: f64,
-    pub channels: Channels,
+    abs_max_tec_i: f64,
 }
 
 impl FanCtrl {
-    pub fn new(mut fan: FanPin, channels: Channels) -> Self {
+    pub fn new(mut fan: FanPin, hwrev: &HWRev) -> Self {
-        let available = channels.hwrev.fan_available();
+        let available = hwrev.fan_available();
 
         if available {
             fan.set_duty(0);
@@ -57,11 +58,12 @@ impl FanCtrl {
             k_a: DEFAULT_K_A,
             k_b: DEFAULT_K_B,
             k_c: DEFAULT_K_C,
-            channels,
+            abs_max_tec_i: 0f64,
         }
     }
 
-    pub fn cycle(&mut self) {
+    pub fn cycle(&mut self, abs_max_tec_i: f64) {
+        self.abs_max_tec_i = abs_max_tec_i;
         self.adjust_speed();
     }
 
@@ -69,7 +71,7 @@ impl FanCtrl {
         if self.available {
             let summary = FanSummary {
                 fan_pwm: self.get_pwm(),
-                abs_max_tec_i: self.channels.current_abs_max_tec_i(),
+                abs_max_tec_i: self.abs_max_tec_i,
                 auto_mode: self.fan_auto,
                 k_a: self.k_a,
                 k_b: self.k_b,
@@ -84,26 +86,23 @@ impl FanCtrl {
 
     pub fn adjust_speed(&mut self) {
         if self.fan_auto && self.available {
-            let scaled_current = self.channels.current_abs_max_tec_i() / MAX_TEC_I;
+            let scaled_current = self.abs_max_tec_i / MAX_TEC_I;
             // do not limit upper bound, as it will be limited in the set_pwm()
             let pwm = (MAX_USER_FAN_PWM * (scaled_current * (scaled_current * self.k_a + self.k_b) + self.k_c)) as u32;
             self.set_pwm(pwm);
         }
     }
 
-    #[inline]
     pub fn set_auto_mode(&mut self, fan_auto: bool) {
         self.fan_auto = fan_auto;
     }
 
-    #[inline]
     pub fn set_curve(&mut self, k_a: f64, k_b: f64, k_c: f64) {
         self.k_a = k_a;
         self.k_b = k_b;
         self.k_c = k_c;
     }
 
-    #[inline]
     pub fn restore_defaults(&mut self) {
         self.set_curve(DEFAULT_K_A, DEFAULT_K_B, DEFAULT_K_C);
     }
@@ -117,7 +116,6 @@ impl FanCtrl {
         value as f64 / (max as f64)
     }
 
-    #[inline]
     fn scale_number(unscaled: f64, to_min: f64, to_max: f64, from_min: f64, from_max: f64) -> f64 {
         (to_max - to_min) * (unscaled - from_min) / (from_max - from_min) + to_min
     }
@@ -125,7 +123,7 @@ impl FanCtrl {
     fn get_pwm(&self) -> u32 {
         let duty = self.fan.get_duty();
         let max = self.fan.get_max_duty();
-        (Self::scale_number(duty as f64 / (max as f64), MIN_USER_FAN_PWM, MAX_USER_FAN_PWM, MIN_FAN_PWM, MAX_FAN_PWM) + 0.5) as u32
+        Self::scale_number(duty as f64 / (max as f64), MIN_USER_FAN_PWM, MAX_USER_FAN_PWM, MIN_FAN_PWM, MAX_FAN_PWM).round() as u32
     }
 }
 
@@ -155,18 +153,3 @@ pub struct FanSummary {
     k_b: f64,
     k_c: f64,
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-
-    #[test]
-    fn test_scaler() {
-        for x in 1..100 {
-            assert_eq!((FanCtrl::scale_number(
-                FanCtrl::scale_number(x as f64, MIN_FAN_PWM, MAX_FAN_PWM, MIN_USER_FAN_PWM, MAX_USER_FAN_PWM),
-                                              MIN_USER_FAN_PWM, MAX_USER_FAN_PWM, MIN_FAN_PWM, MAX_FAN_PWM) + 0.5) as i32,
-                       x);
-        }
-    }
-}

src/main.rs

@@ -150,7 +150,7 @@ fn main() -> ! {
         }
     }
 
-    let mut fan_ctrl = FanCtrl::new(fan, channels);
+    let mut fan_ctrl = FanCtrl::new(fan, &channels.hwrev);
 
     // default net config:
     let mut ipv4_config = Ipv4Config {
@@ -180,12 +180,12 @@ fn main() -> ! {
             loop {
                 let mut new_ipv4_config = None;
                 let instant = Instant::from_millis(i64::from(timer::now()));
-                let updated_channel = fan_ctrl.channels.poll_adc(instant);
+                let updated_channel = channels.poll_adc(instant);
                 if let Some(channel) = updated_channel {
                     server.for_each(|_, session| session.set_report_pending(channel.into()));
                 }
 
-                fan_ctrl.cycle();
+                fan_ctrl.cycle(channels.current_abs_max_tec_i());
 
                 let instant = Instant::from_millis(i64::from(timer::now()));
                 cortex_m::interrupt::free(net::clear_pending);
@@ -210,7 +210,7 @@ fn main() -> ! {
                                 // Do nothing and feed more data to the line reader in the next loop cycle.
                                 Ok(SessionInput::Nothing) => {}
                                 Ok(SessionInput::Command(command)) => {
-                                    match Handler::handle_command(command, &mut socket, session, &mut leds, &mut store, &mut ipv4_config, &mut fan_ctrl) {
+                                    match Handler::handle_command(command, &mut socket, &mut channels, session, &mut leds, &mut store, &mut ipv4_config, &mut fan_ctrl) {
                                         Ok(Handler::NewIPV4(ip)) => new_ipv4_config = Some(ip),                                
                                         Ok(Handler::Handled) => {},
                                         Ok(Handler::CloseSocket) => socket.close(),
@@ -227,7 +227,7 @@ fn main() -> ! {
                             }
                         } else if socket.can_send() {
                             if let Some(channel) = session.is_report_pending() {
-                                match fan_ctrl.channels.reports_json() {
+                                match channels.reports_json() {
                                     Ok(buf) => {
                                         send_line(&mut socket, &buf[..]);
                                         session.mark_report_sent(channel);

src/pins.rs

@@ -26,7 +26,7 @@ use stm32f4xx_hal::{
         TIM1, TIM3, TIM8
     },
     timer::Timer,
-    time::{U32Ext, KiloHertz},
+    time::U32Ext,
 };
 use eeprom24x::{self, Eeprom24x};
 use stm32_eth::EthPins;
@@ -36,8 +36,6 @@ use crate::{
     fan_ctrl::FanPin
 };
 
-const PWM_FREQ: KiloHertz = KiloHertz(20u32);
-
 pub type Eeprom = Eeprom24x<
     I2c<I2C1, (
         PB8<AlternateOD<{ stm32f4xx_hal::gpio::AF4 }>>,
@@ -228,7 +226,11 @@ impl Pins {
             hclk: clocks.hclk(),
         };
 
-        let fan = Timer::new(tim8, &clocks).pwm(gpioc.pc9.into_alternate(), PWM_FREQ);
+        // Though there is not enough evidence available for concrete fan model,
+        // it is generally advised to have higher PWM frequencies for brushless motors,
+        // so that it would produce less audible noise.
+        // Source: https://www.controleng.com/articles/understanding-the-effect-of-pwm-when-controlling-a-brushless-dc-motor/
+        let fan = Timer::new(tim8, &clocks).pwm(gpioc.pc9.into_alternate(), 80u32.khz());
 
         (pins, leds, eeprom, eth_pins, usb, fan)
     }
@@ -312,6 +314,7 @@ impl PwmPins {
         max_i_neg0: PE13<M5>,
         max_i_neg1: PE14<M6>,
     ) -> PwmPins {
+        let freq = 20u32.khz();
 
         fn init_pwm_pin<P: hal::PwmPin<Duty=u16>>(pin: &mut P) {
             pin.set_duty(0);
@@ -321,8 +324,8 @@ impl PwmPins {
             max_v0.into_alternate(),
             max_v1.into_alternate(),
         );
-        //let (mut max_v0, mut max_v1) = pwm::tim3(tim3, channels, clocks, PWM_FREQ);
+        //let (mut max_v0, mut max_v1) = pwm::tim3(tim3, channels, clocks, freq);
-        let (mut max_v0, mut max_v1) = Timer::new(tim3, &clocks).pwm(channels, PWM_FREQ);
+        let (mut max_v0, mut max_v1) = Timer::new(tim3, &clocks).pwm(channels, freq);
         init_pwm_pin(&mut max_v0);
         init_pwm_pin(&mut max_v1);
 
@@ -333,7 +336,7 @@ impl PwmPins {
             max_i_neg1.into_alternate(),
         );
         let (mut max_i_pos0, mut max_i_pos1, mut max_i_neg0, mut max_i_neg1) =
-            Timer::new(tim1, &clocks).pwm(channels, PWM_FREQ);
+            Timer::new(tim1, &clocks).pwm(channels, freq);
         init_pwm_pin(&mut max_i_pos0);
         init_pwm_pin(&mut max_i_neg0);
         init_pwm_pin(&mut max_i_pos1);