M-Labs/thermostat
9
0
Fork 5
Code Issues 9 Pull Requests 5 Releases Wiki Activity

Add swap command #104

Closed
atse wants to merge 2 commits from atse:swap_tec_polarity into master
pull from: atse:swap_tec_polarity
merge into: M-Labs:master
Conversation 2 Commits 2 Files Changed 6 +59 -6
atse commented 2024-04-26 17:44:37 +08:00
Contributor
Copy Link

For use with Zotino, since the Zotino header on the Thermostat has the current pins reversed.

For use with Zotino, since the Zotino header on the Thermostat has the current pins reversed.
atse added 2 commits 2024-04-26 17:44:37 +08:00 
Per-channel current swapping, for use with Zotino in the meantime on
HWRevs < 3.0 as the Zotino headers are swapped on the Thermostat.
Signed-off-by: Egor Savkin <es@m-labs.hk>
esavkin reviewed 2024-04-29 10:41:52 +08:00
src/command_handler.rs
@ -414,1 +414,4 @@
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) {
esavkin commented 2024-04-29 10:41:52 +08:00
Owner
Copy Link

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

Isn't there a way to set it directly into channel_state?
sb10q reviewed 2024-04-29 13:22:46 +08:00
src/channels.rs
@ -122,0 +122,4 @@
-i_set
} else {
i_set
}
sb10q commented 2024-04-29 13:22:46 +08:00
Owner
Copy Link

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?
sb10q reviewed 2024-04-29 13:26:20 +08:00
src/command_handler.rs
@ -415,0 +415,4 @@
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) {
channels.channel_state(c).swap_tec_polarity = !channels.channel_state(c).swap_tec_polarity;
sb10q commented 2024-04-29 13:26:20 +08:00
Owner
Copy Link

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.
atse commented 2024-07-05 16:51:02 +08:00
Author
Contributor
Copy Link

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.
atse commented 2024-07-05 16:52:01 +08:00
Author
Contributor
Copy Link

Closing this PR in favour of a mode switching command instead.

Closing this PR in favour of a mode switching command instead.
atse closed this pull request 2024-07-05 16:52:01 +08:00
sb10q commented 2024-07-07 14:19:07 +08:00
Owner
Copy Link

Please don't call it a Zotino mode. It's a terrible name that gives no indication about what it does and can be misleading in many ways (e.g. does it set PID parameters for Zotino?).

Please don't call it a Zotino mode. It's a terrible name that gives no indication about what it does and can be misleading in many ways (e.g. does it set PID parameters for Zotino?).
👍 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.
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
3 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#104
No description provided.
Delete Branch "atse:swap_tec_polarity"

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?