Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

rpv-tomsk
Copy link
Contributor

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

@rpv-tomsk
BL0937: Fixed swapped division and multiplication in measured value c…
2ad71ec 
…orrection

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.
@openshwprojects
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

rpv-tomsk commented Jun 25, 2024 •
edited
Loading

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect calculations for BL0937 power meter pulses
2 participants
@rpv-tomsk @openshwprojects