dac_fix and review #52
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "dac_fix"
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?
Contains fix for inconsistent output current behavior, snuck in minor improvements added during code review.
@ -180,6 +180,10 @@ postfilter rate can be tuned with the `postfilter` command.
- Connect Peliter device 1 to TEC1- and TEC1+.
- The GND pin is for shielding not for sinking Peltier currents.
When using a TEC module with the Thermostat, the Thermostat expects the thermal load (where the thermistor is located) to heat up with a positive software current set point, and cool down with a negative current set point.
The doc sometimes says "TEC module" and sometimes "Peltier device"; we should use consistent terminology.
@ -114,3 +112,2 @@
};
let voltage = self.channel_state(channel).dac_value;
let max = ElectricPotential::new::<volt>(ad5680::MAX_VALUE as f64 / dac_factor);
let max = ElectricPotential::new::<volt>(DAC_OUT_MAX_V);
Why do we need to return this if it's constant? Check what is using
get_dac
and see if that abstraction is relevant.@ -23,2 +23,4 @@
pub const CHANNELS: usize = 2;
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
pub const DAC_OUT_MAX_V: f64 = 3.0;
Can't have
ElectricPotential
constants?One purpose of the uom crate is to have the type checker verify dimensional homogeneity; that only works if used consistently.
@ -260,7 +256,12 @@ impl Channels {
/// These loops perform a breadth-first search for the DAC setting
/// that will produce a `target_voltage`.
Comments should explain the STM ADC offset compensation technique.
@ -348,3 +349,2 @@
pub fn get_max_v(&mut self, channel: usize) -> (ElectricPotential, ElectricPotential) {
let vref = self.channel_state(channel).vref;
let max = 4.0 * vref;
let max = 4.0 * ElectricPotential::new::<volt>(3.3);
Same as above, check if the abstraction is relevant and it makes sense to return a constant.
@ -263,1 +259,3 @@
let target_voltage = ElectricPotential::new::<volt>(2.5);
let samples = 50;
let mut target_voltage = ElectricPotential::new::<volt>(0.0);
for _ in (0..samples).rev() {
.rev()
?@ -260,7 +256,12 @@ impl Channels {
/// These loops perform a breadth-first search for the DAC setting
Do we really need a breadth-first search, or is it sufficient (e.g. fast enough) to simply increment (or decrement depending on the sign of the error) the DAC value until the ADC reads back Vref?
The time it takes is not noticeable.
OK, then a simple increment/decrement is better (simpler) and then the code comments can focus solely on the STM ADC offset.
Misunderstood what you said there. I think if the search works and is fast, then probably should leave it there.
@ -153,3 +149,2 @@
let i_tec = (voltage - center_point) / (10.0 * r_sense);
let max = (max - center_point) / (10.0 * r_sense);
(i_tec, max)
// let max = (max - center_point) / (10.0 * r_sense);
Remove?
@ -262,0 +256,4 @@
/// The thermostat DAC applies a control voltage signal to the CTLI pin of MAX driver chip to control output current.
/// The CTLI input signal is centered around VREF of the MAX chip. Applying VREF to CTLI sets the output current to 0.
///
/// This calibration routine reads the VREF voltage, and finds a DAC output value that would
...reads the VREF voltage and the DAC output by using the STM32 ADC...
@ -1,6 +1,6 @@
#![cfg_attr(not(test), no_std)]
#![cfg_attr(not(test), no_main)]
#![feature(maybe_uninit_extra, maybe_uninit_ref, asm)]
#![feature(maybe_uninit_extra, maybe_uninit_ref, asm, const_fn)]
We won't need const_fn anymore, right?
@ -1,4 +1,8 @@
use stm32f4xx_hal::hal::digital::v2::OutputPin;
use uom::si::{
f64::{ElectricPotential},
Braces are not necessary here.
8971ffc89c
toc23ffd70e2
c23ffd70e2
to8ca8c0b31c
8ca8c0b31c
to011433c200
011433c200
to46136942af
46136942af
to6b9d61737e