openshwprojects / OpenBK7231T_App

Open source firmware (Tasmota/Esphome replacement) for BK7231T, BK7231N, BL2028N, T34, XR809, W800/W801, W600/W601 and BL602
https://openbekeniot.github.io/webapp/devicesList.html
1.33k stars 226 forks source link

BL0937: Fixed swapped division and multiplication in measured value correction #1280

Open rpv-tomsk opened 4 days ago

rpv-tomsk commented 4 days ago

value = pulses / time

More pulses for same time - bigger value. More time for same pulses count - smaller value. Twice more pulses for twice time - same value.

ticksElapased tick_period == elapsed time in ms ticksElapased tick_period / 1000 == elapsed time in s

final = final expected_time / elapsed_time; //expected time is 1 second. final = final expected_ticks / elapsed_ticks; //expected ticks for 1 second: (1000.0ms / portTICK_PERIOD_MS)

Closes: #1270

openshwprojects commented 4 days ago

I will need to... think about it. I am not the author of that division for BL0937. Would it be possible to somehow... make an experiment to verify which version is correct? Hm, I could try ticking drivers every 2 seconds, not 1.... and then see..

rpv-tomsk commented 3 days ago

I don't think experiment is needed, because of clear principle of work: the frequency is proportional to the value. More the value - more pulses per fixed unit of time. Pure math.

rpv-tomsk commented 3 days ago

Hm, I could try ticking drivers every 2 seconds, not 1.... and then see..

Yes, that should produce 4x greater measurement. 2x - twice the counted pulses and 2x - from ticksElapsed. But correct implementation should leave result unchanged and independent from ticks period.

rpv-tomsk commented 3 days ago

Another issue is overall 'ticks correction' / PwrCal_Scale() design. We have to use current/voltage measurement unchanged from previous cycle, but implementation does correction for all (Voltage and Current) values. Due to ticksElapsed mismatch between cycles, that produces additional error in measurement. This is hard to fix due to PwrCal_Scale() always called for all three values, while we need to leave one of final_v or final_c left untouched. To solve this we need to have separated PwrCal_ScaleVoltage, PwrCal_ScaleCurrent and PwrCal_ScalePower.