Add swap command #104

Closed
atse wants to merge 2 commits from atse:swap_tec_polarity into master
6 changed files with 57 additions and 6 deletions
Showing only changes of commit 6fadfc0b49 - Show all commits

View File

@ -130,6 +130,7 @@ formatted as line-delimited JSON.
| `fcurve <a> <b> <c>` | Set fan controller curve coefficients (see *Fan control* section) | | `fcurve <a> <b> <c>` | Set fan controller curve coefficients (see *Fan control* section) |
| `fcurve default` | Set fan controller curve coefficients to defaults (see *Fan control* section) | | `fcurve default` | Set fan controller curve coefficients to defaults (see *Fan control* section) |
| `hwrev` | Show hardware revision, and settings related to it | | `hwrev` | Show hardware revision, and settings related to it |
| `swap [0/1]` | Swap TEC polarities for channel all/0/1 (For use with Zotino) |
## USB ## USB
@ -180,9 +181,11 @@ postfilter rate can be tuned with the `postfilter` command.
## Thermo-Electric Cooling (TEC) ## Thermo-Electric Cooling (TEC)
- Connect TEC module device 0 to TEC0- and TEC0+. - Connect TEC module device 0 to TEC0-/T0- and TEC0+/T0+.
- Connect TEC module device 1 to TEC1- and TEC1+. - Connect TEC module device 1 to TEC1-/T1- and TEC1+/T1+.
- The GND pin is for shielding not for sinking TEC module currents. - The GND pin is for shielding, not for sinking TEC module currents.
For Zotino temperature regulation, connect an IDC cable to the internal Zotino header labeled TEC1/TEC2. For pre 3.0 Thermostats, use the `swap` command for those channels.
When using a TEC module with the Thermostat, the Thermostat expects the thermal load (where the thermistor is located) to cool down with a positive software current set point, and heat up with a negative current set point. When using a TEC module with the Thermostat, the Thermostat expects the thermal load (where the thermistor is located) to cool down with a positive software current set point, and heat up with a negative current set point.

View File

@ -35,6 +35,7 @@ pub struct ChannelState {
pub pid_engaged: bool, pub pid_engaged: bool,
pub pid: pid::Controller, pub pid: pid::Controller,
pub sh: sh::Parameters, pub sh: sh::Parameters,
pub swap_tec_polarity: bool,
} }
impl ChannelState { impl ChannelState {
@ -51,6 +52,7 @@ impl ChannelState {
pid_engaged: false, pid_engaged: false,
pid: pid::Controller::new(pid::Parameters::default()), pid: pid::Controller::new(pid::Parameters::default()),
sh: sh::Parameters::default(), sh: sh::Parameters::default(),
swap_tec_polarity: false,
} }
} }

View File

@ -118,8 +118,12 @@ impl<'a> Channels<'a> {
pub fn get_i(&mut self, channel: usize) -> ElectricCurrent { pub fn get_i(&mut self, channel: usize) -> ElectricCurrent {
let i_set = self.channel_state(channel).i_set; let i_set = self.channel_state(channel).i_set;
if self.channel_state(channel).swap_tec_polarity {
-i_set
} else {
i_set i_set
} }
Review

Can't it be handled just at the DAC and current readout, instead of having to invert it all over the place?

Can't it be handled just at the DAC and current readout, instead of having to invert it all over the place?
}
/// i_set DAC /// i_set DAC
fn set_dac(&mut self, channel: usize, voltage: ElectricPotential) -> ElectricPotential { fn set_dac(&mut self, channel: usize, voltage: ElectricPotential) -> ElectricPotential {
@ -137,13 +141,16 @@ impl<'a> Channels<'a> {
// Silently clamp i_set // Silently clamp i_set
let i_ceiling = ElectricCurrent::new::<ampere>(MAX_TEC_I); let i_ceiling = ElectricCurrent::new::<ampere>(MAX_TEC_I);
let i_floor = ElectricCurrent::new::<ampere>(-MAX_TEC_I); let i_floor = ElectricCurrent::new::<ampere>(-MAX_TEC_I);
let i_set = i_set.min(i_ceiling).max(i_floor); let mut i_set = i_set.min(i_ceiling).max(i_floor);
let vref_meas = match channel.into() { let vref_meas = match channel.into() {
0 => self.channel0.vref_meas, 0 => self.channel0.vref_meas,
1 => self.channel1.vref_meas, 1 => self.channel1.vref_meas,
_ => unreachable!(), _ => unreachable!(),
}; };
if self.channel_state(channel).swap_tec_polarity {
i_set = -i_set;
}
let center_point = vref_meas; let center_point = vref_meas;
let r_sense = ElectricalResistance::new::<ohm>(R_SENSE); let r_sense = ElectricalResistance::new::<ohm>(R_SENSE);
let voltage = i_set * 10.0 * r_sense + center_point; let voltage = i_set * 10.0 * r_sense + center_point;
@ -390,7 +397,12 @@ impl<'a> Channels<'a> {
// Get current passing through TEC // Get current passing through TEC
pub fn get_tec_i(&mut self, channel: usize) -> ElectricCurrent { pub fn get_tec_i(&mut self, channel: usize) -> ElectricCurrent {
(self.read_itec(channel) - self.read_vref(channel)) / ElectricalResistance::new::<ohm>(0.4) let tec_i = (self.read_itec(channel) - self.read_vref(channel)) / ElectricalResistance::new::<ohm>(0.4);
if self.channel_state(channel).swap_tec_polarity {
-tec_i
} else {
tec_i
}
} }
// Get voltage across TEC // Get voltage across TEC

View File

@ -412,6 +412,16 @@ impl Handler {
} }
} }
fn swap_tec_polarity (socket: &mut TcpSocket, channels: &mut Channels, channel: Option<usize>) -> Result<Handler, Error> {
for c in 0..CHANNELS {
if channel.is_none() || channel == Some(c) {
Review

Isn't there a way to set it directly into channel_state?

Isn't there a way to set it directly into channel_state?
channels.channel_state(c).swap_tec_polarity = !channels.channel_state(c).swap_tec_polarity;
Review

That's a terrible API. Simply add a way to set swapped to true/false (with the reference being the front panel polarity) instead of only something that toggles the property without any way to know what its value is.

That's a terrible API. Simply add a way to set swapped to true/false (with the reference being the front panel polarity) instead of only something that toggles the property without any way to know what its value is.
Review

Yeah... to tell the truth this was a bodged API intended to get temperature regulation on the Zotino DAC working quickly; This is sketchy for production.

I've been planning an output mode command that could switch between TEC and resistive heater modes, and think adding a Zotino mode to that would be a good fit, since all 3 modes are about the current direction to the load. That's most certainly better than just a swap command API-wise, so I think we should opt for that instead.

Yeah... to tell the truth this was a bodged API intended to get temperature regulation on the Zotino DAC working quickly; This is sketchy for production. I've been planning an output mode command that could switch between TEC and resistive heater modes, and think adding a Zotino mode to that would be a good fit, since all 3 modes are about the current direction to the load. That's most certainly better than just a swap command API-wise, so I think we should opt for that instead.
}
}
send_line(socket, b"{}");
Ok(Handler::Handled)
}
pub fn handle_command(command: Command, socket: &mut TcpSocket, channels: &mut Channels, session: &Session, store: &mut FlashStore, ipv4_config: &mut Ipv4Config, fan_ctrl: &mut FanCtrl, hwrev: HWRev) -> Result<Self, Error> { pub fn handle_command(command: Command, socket: &mut TcpSocket, channels: &mut Channels, session: &Session, store: &mut FlashStore, ipv4_config: &mut Ipv4Config, fan_ctrl: &mut FanCtrl, hwrev: HWRev) -> Result<Self, Error> {
match command { match command {
Command::Quit => Ok(Handler::CloseSocket), Command::Quit => Ok(Handler::CloseSocket),
@ -441,6 +451,7 @@ impl Handler {
Command::FanCurve { k_a, k_b, k_c } => Handler::fan_curve(socket, fan_ctrl, k_a, k_b, k_c), Command::FanCurve { k_a, k_b, k_c } => Handler::fan_curve(socket, fan_ctrl, k_a, k_b, k_c),
Command::FanCurveDefaults => Handler::fan_defaults(socket, fan_ctrl), Command::FanCurveDefaults => Handler::fan_defaults(socket, fan_ctrl),
Command::ShowHWRev => Handler::show_hwrev(socket, hwrev), Command::ShowHWRev => Handler::show_hwrev(socket, hwrev),
Command::SwapTECPolarity { channel } => Handler::swap_tec_polarity(socket, channels, channel),
} }
} }
} }

View File

@ -191,6 +191,9 @@ pub enum Command {
}, },
FanCurveDefaults, FanCurveDefaults,
ShowHWRev, ShowHWRev,
SwapTECPolarity {
channel: Option<usize>,
},
} }
fn end(input: &[u8]) -> IResult<&[u8], ()> { fn end(input: &[u8]) -> IResult<&[u8], ()> {
@ -584,6 +587,22 @@ fn fan_curve(input: &[u8]) -> IResult<&[u8], Result<Command, Error>> {
))(input) ))(input)
} }
fn swap(input: &[u8]) -> IResult<&[u8], Result<Command, Error>> {
let (input, _) = tag("swap")(input)?;
let (input, channel) = alt((
|input| {
let (input, _) = whitespace(input)?;
let (input, channel) = channel(input)?;
let (input, _) = end(input)?;
Ok((input, Some(channel)))
},
value(None, end)
))(input)?;
let result = Ok(Command::SwapTECPolarity { channel });
Ok((input, result))
}
fn command(input: &[u8]) -> IResult<&[u8], Result<Command, Error>> { fn command(input: &[u8]) -> IResult<&[u8], Result<Command, Error>> {
alt((value(Ok(Command::Quit), tag("quit")), alt((value(Ok(Command::Quit), tag("quit")),
load, load,
@ -600,6 +619,7 @@ fn command(input: &[u8]) -> IResult<&[u8], Result<Command, Error>> {
fan, fan,
fan_curve, fan_curve,
value(Ok(Command::ShowHWRev), tag("hwrev")), value(Ok(Command::ShowHWRev), tag("hwrev")),
swap,
))(input) ))(input)
} }

View File

@ -22,6 +22,7 @@ pub struct ChannelConfig {
pwm: PwmLimits, pwm: PwmLimits,
/// uses variant `PostFilter::Invalid` instead of `None` to save space /// uses variant `PostFilter::Invalid` instead of `None` to save space
adc_postfilter: PostFilter, adc_postfilter: PostFilter,
swap_tec_polarity: bool,
} }
impl ChannelConfig { impl ChannelConfig {
@ -41,6 +42,7 @@ impl ChannelConfig {
sh: state.sh.clone(), sh: state.sh.clone(),
pwm, pwm,
adc_postfilter, adc_postfilter,
swap_tec_polarity: state.swap_tec_polarity,
} }
} }
@ -51,6 +53,7 @@ impl ChannelConfig {
state.pid.target = self.pid_target.into(); state.pid.target = self.pid_target.into();
state.pid_engaged = self.pid_engaged; state.pid_engaged = self.pid_engaged;
state.sh = self.sh.clone(); state.sh = self.sh.clone();
state.swap_tec_polarity = self.swap_tec_polarity;
self.pwm.apply(channels, channel); self.pwm.apply(channels, channel);