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

PID_improvements #45

Merged
sb10q merged 7 commits from PID_improvements into master 2021-01-11 20:29:51 +08:00
Conversation 0 Commits 7 Files Changed 4
topquark12 commented 2021-01-11 14:09:24 +08:00

Changes:

  • Rescale integral when ki is changed to prevent kick when tuning ki.

  • Use safer default PID parameters.

  • When reporting, tec v measurement shows voltage across tec, not voltage at adc pin

  • Added methods to retrieve TEC voltage and current

  • Integral anti windup when actual TEC current deviates from TEC current setpoint (compliance voltage reached)

Changes: - Rescale integral when ki is changed to prevent kick when tuning ki. - Use safer default PID parameters. - When reporting, tec v measurement shows voltage across tec, not voltage at adc pin - Added methods to retrieve TEC voltage and current - Integral anti windup when actual TEC current deviates from TEC current setpoint (compliance voltage reached)
topquark12 added 1 commit 2021-01-11 14:10:55 +08:00
sb10q reviewed 2021-01-11 16:13:08 +08:00
src/channel_state.rs
@ -3,6 +3,7 @@ use uom::si::{
f64::{
ElectricPotential,
ElectricalResistance,
sb10q commented 2021-01-11 16:13:08 +08:00
Owner

testing if error 500 is fixed

testing if error 500 is fixed
sb10q commented 2021-01-11 16:13:14 +08:00
Owner

seems ok

seems ok
sb10q commented 2021-01-11 16:13:21 +08:00
Owner

still ok

still ok
sb10q marked this conversation as resolved
sb10q reviewed 2021-01-11 16:13:58 +08:00
src/channels.rs Outdated
@ -364,6 +364,16 @@ impl Channels {
(duty * max, max)
}
// Get actual current passing through TEC
sb10q commented 2021-01-11 16:13:58 +08:00
Owner

Actual?

Actual?
topquark12 commented 2021-01-11 16:18:41 +08:00
Poster

There are quite a few variables describing currents, like current setpoint, dac voltage setpoint controlling output current, dac voltage readback for current control.

This returns the thermostat measured current that is passing through the TEC, hence "actual".

There are quite a few variables describing currents, like current setpoint, dac voltage setpoint controlling output current, dac voltage readback for current control. This returns the thermostat measured current that is passing through the TEC, hence "actual".
topquark12 commented 2021-01-11 16:23:11 +08:00
Poster

I'll rephrase the comment for clarity.

I'll rephrase the comment for clarity.
topquark12 added 1 commit 2021-01-11 16:24:50 +08:00
sb10q reviewed 2021-01-11 16:29:30 +08:00
src/pid.rs
@ -5,2 +6,4 @@
};
/// Allowable current error for integral accumulation
const CURRENT_ERROR_MAX: f64 = 0.1;
sb10q commented 2021-01-11 16:29:30 +08:00
Owner

Shouldn't this be configurable as well? The situation here doesn't seem very different than the configurable integral clipping.

Shouldn't this be configurable as well? The situation here doesn't seem very different than the configurable integral clipping.
topquark12 commented 2021-01-11 16:34:00 +08:00
Poster

There isn't really a lot of reason for this to be user configurable I think. It mostly has to do with the inherrent noise of the current reading, which will be pretty consistent between other thermostat devices.

TBH I don't really think the integral clipping makes a lot of sense as well, as it heavily depends on the ki value and ambient temperature, thermal load, and usually I just set it so large so that it doesn't get into the way.

There isn't really a lot of reason for this to be user configurable I think. It mostly has to do with the inherrent noise of the current reading, which will be pretty consistent between other thermostat devices. TBH I don't really think the integral clipping makes a lot of sense as well, as it heavily depends on the ki value and ambient temperature, thermal load, and usually I just set it so large so that it doesn't get into the way.
sb10q commented 2021-01-11 20:28:50 +08:00
Owner

Should the clipping be hardcoded as well then (and perhaps be scaled by ki)?

Should the clipping be hardcoded as well then (and perhaps be scaled by ki)?
topquark12 commented 2021-01-12 15:32:01 +08:00
Poster

I'll see if it makes sense to calculate integral limit based on parameters during the code review.

I'll see if it makes sense to calculate integral limit based on parameters during the code review.
sb10q merged commit 9e4d06fdbc into master 2021-01-11 20:29:51 +08:00
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
2 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#45
There is no content yet.
Delete Branch "PID_improvements"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?