Pinecil v2 tune via PID

[Ralim]

• Please check if the PR fulfills these requirements

• The changes have been tested locally
• [] There are no breaking changes

• What kind of change does this PR introduce?

Basically, I've given up with the previous control scheme and have written a fairly trivial PID implementation and tuned it for my PinecilV2.

This includes:

• Made a workaround for glitchy / missed timer events
• Wrote basic PID
• Opted PinecilV2 into said PID
• Tuned PinecilV2 for said PID

• What is the current behavior?

#1824
#1688

• What is the new behavior (if this is a feature change)?

• Other information:
  This still isnt perfect but for my testing is largely improved. Any testing would be highly appreciated. If you still get the large oscillations please let me know what tip + power supply is being used so I can attempt to replicate.

[discip]

These screenshots speak for themselves!

[Ralim]

@discip
Added a small filter over the displayed value on the gui to reduce flicker; curious your thoughts?

Still more to tune on this, but that will likely be 2.23

[discip]

I',m speechless.
I already started to type the request for adding a tranquilizer to the display, but then scrapped it so as not to seem ungrateful. 😀

Unfortunately not everything is covered by this as I've seen spikes of up to ~334°C (@ 320°C).

[Ralim]

I said I would get it eventually; life just got in the way. Finally trying to make back lost time.
Its all good, you know I value input.

By speeding up the PID; I made the screen a little aggressive 🤣 So while I still stand by the idea of "dont filter out the real world" like some devices do, this makes a lot of sense.

I've spotted the odd spike too, I need to look into them, I suspect its due to the a quirk with me skipping a cycle when the timer does weird things -- more debugging required to really know for certain.

[goga1978]

And if you use a median filter instead of a moving average?

[Ralim]

The type of filter in this instance wont really change things as the filter is really only going to average 1-2 readings at a time (and just saturate at the new reading between). The outliers mentioned above are real and are a quirk of supressing the adc cycle. I need to prevent the spike rather than filter it from view.

[goga1978]

moving average
[290, 300, 380] = 970 / 3 = 323

median filter
[290, 300, 380] = 300

[Ralim]

I'm aware of how a median filter works, but its not the best solution as our data looks like:

{319,319,319,319,319,319,321,321,321,321,321,321,318,318,318,318,318}
Or in the case of a spike
{319,319,319,319,319,319,340,340,340,340,340,340,318,318,318,318,318}

As its called per gui redraw.
Also a median filter is not O(1) as far as I recall, and O(n) and above is nice to avoid in the fast loop for the gui.

I'll just focus on preventing the spike in the first place :)

[goga1978]

median filter
{319,319,319,319,319,319,340,340,340,340,340,340,318,318,318,318,318}
sort
{318,318,318,318,318,319,319,319,319,319,319,340,340,340,340,340,340} = x[8] = 319
with a surge , there will be such a value

sorted in ascending order and the middle is taken

[Ralim]

@discip I've done a small tweak to hopefully reduce the spikes a little.
See how that looks; I think this should be ok to merge now unless you spot any further issues 😓

@goga1978
yes, but a sort is not O(1) time to run.
Secondly of that dataset, only a sliding window of ~8 readings is being used at a time; the goal is not to hide the spike.
The goal is to prevent small jitter, so the spike should still be visible if it occurs.

[discip]

See how that looks;

Will do as soon as I'm back home. 😅

[discip]

1. I noticed the iron to display 1°C less than PineSAM
2. Unfortunately the spikes still occur (334°C @ 320°C)!
   The graph does look unchanged, almost stationary.

[goga1978]

HI.
https://files.pine64.org/doc/Pinecil/Pinecil_schematic_v2.0_20220608.pdf
I looked at the soldering iron diagram, it's terrible. Marked with red circles.

Soldering iron tip-

The amplifier for the sting thermocouple. Connects to point A. Attention at SGM8557 - there is no protection of the entrance!!! Do not turn on without a sting!!!

The output of the SGM8557 amplifier is connected to Tip_Sence.

On the Tip_Sence pin there is a capacitor C22 with a capacity of 22000pf. So the first values in the interval between PWMs will be overestimated !!!

[goga1978]

HI.
FILE IRQ.cpp
// Returns the current raw tip reading after any cleanup filtering
// For Pinecil V2 we dont do any rolling filtering other than just averaging all 4 readings in the adc snapshot
uint16_t getTipRawTemp(uint8_t sample) { return ADC_Tip.average() >> 1;

where is it set ? that only 4 temperature values will be recorded?

[goga1978]

how do I skip the first two values? And write down the following ?
Constantly skip the first two values.
so that the capacitor c22 has time to discharge

[goga1978]

diode limitation input voltage up to 0.5 volts

[Ralim]

@goga1978 This is off topic to this PR, please use a suitable thread.

The input to the op-amp is fine; the 100K resistor limits our current to 280uA max which is handled fine by the internal clamping diodes. As confirmed by manufacturer.

so that the capacitor c22 has time to discharge

C22 is not a problem as we wait for the charge change before we start the ADC, and it helps keep readings stable while the ADC is charging its internal measurement cap. There is no requirement to discard readings. The delay is configured via the holdoffTicks variable. This is adjusted in the functions for changing PWM rate for example here

where is it set ? that only 4 temperature values will be recorded?

In your IDE, if you look at references to the ADC_Tip variable you will find it is updated in the IRQ handler near the top of the file.

[Ralim]

@discip

1. I noticed the iron to display 1°C less than PineSAM

   2. Unfortunately the spikes still occur (334°C @ 320°C)!
      The graph does look unchanged, almost stationary.

Ah, PineSAM is not affected by any GUI filtering I do. Bluetooth reads directly with no filters, so that its up to app developers how they want to handle that

Dang on the spike still occuring. Ill spend a bit longer trying to reproduce them.
I might merge this in as-is and look at spikes in a new PR so this doesnt get too big of a change 😓

[goga1978]

Hi

#define ADC_Filter_Smooth 1 /* This basically smooths over one PWM cycle / set of readings */

void switchToFastPWM(void) {
inFastPWMMode = true;
holdoffTicks = 12;
tempMeasureTicks = 10;
totalPWM = powerPWM + tempMeasureTicks + holdoffTicks;
TIMER_SetCompValue(TIMER_CH0, TIMER_COMP_ID_2, totalPWM);

// ~10Hz
TIMER_SetCompValue(TIMER_CH0, TIMER_COMP_ID_0, powerPWM + holdoffTicks);

// Set divider to 10 ~= 10.5Hz
uint32_t tmpVal = BL_RD_REG(TIMER_BASE, TIMER_TCDR);

tmpVal = BL_SET_REG_BITS_VAL(tmpVal, TIMER_TCDR2, 20);

BL_WR_REG(TIMER_BASE, TIMER_TCDR, tmpVal);
}

void switchToSlowPWM(void) {
// 5Hz
inFastPWMMode = false;
holdoffTicks = 12;
tempMeasureTicks = 10;
totalPWM = powerPWM + tempMeasureTicks + holdoffTicks;

TIMER_SetCompValue(TIMER_CH0, TIMER_COMP_ID_2, totalPWM);
// Adjust ADC
TIMER_SetCompValue(TIMER_CH0, TIMER_COMP_ID_0, powerPWM + holdoffTicks);

// Set divider for ~ 5Hz

uint32_t tmpVal = BL_RD_REG(TIMER_BASE, TIMER_TCDR);

tmpVal = BL_SET_REG_BITS_VAL(tmpVal, TIMER_TCDR2, 20);

BL_WR_REG(TIMER_BASE, TIMER_TCDR, tmpVal);
}

I did so and it seems like the races were gone )))

[discip]

Ah, PineSAM is not affected by any GUI filtering I do. Bluetooth reads directly with no filters,

I somehow assumed that to be the case. 😊

Ill spend a bit longer trying to reproduce them.

👍🏻