-
Notifications
You must be signed in to change notification settings - Fork 274
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.
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
Voltage-Compensated Dummy Load Power Measurement #2810
base: master
Are you sure you want to change the base?
Voltage-Compensated Dummy Load Power Measurement #2810
Conversation
13aacb3
to
89d92b9
Compare
@bramstroker I just checked with the suggested method of using an incandescent light bulb as dummy load. Granted, I don't have one, but I had one of these regular shaped light bulbs with a Halogen Bulb inside. Waiting one minute to let it heat up, and the script will be happy on it's first try and determines, that the dummy load has sufficiently heated. So they work fine as dummy load. Measuring a dummy load with the script and save the resistance after it has stabilized. Then abort and wait two hours and let it measure, but do not attach a light will lead to ±0.02 W noise on the standard settings ( |
9b4d904
to
a180936
Compare
@RubenKelevra thanks. Will review in a few days. |
a180936
to
2edbbc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! just did a first quick glance. here are my first comments. Will do a more throrough review later on
4ec7baa
to
f2cc9f5
Compare
Probably missed a couple of things now, and no testing yet. Will have another look at this tomorrow or at the weekend |
356ac0f
to
037f68f
Compare
Testing complete. Should work now for HA and Shelly V2/3 devices. It reads the new .env var successfully, does auto detect and if this fails asks the user to select the voltagemeter. |
7174430
to
9360a7d
Compare
@RubenKelevra before merging, ruff linting has to be fixed, see failed action. |
Yeah ruff fails for the fixmes, and makes 2-3 suggestions which IMHO reduce readability. You really want those changes to be made? Tests I'll look later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my suggestions how to both simplify the code and make ruff happy.
For the fix me you can add ignores # noqa: FIX001
, that's the annotation you can use to ignore ruff errors.
Also see https://docs.astral.sh/ruff/linter/#error-suppression
Co-authored-by: Bram Gerritsen <[email protected]>
Co-authored-by: Bram Gerritsen <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
94ed399
to
8c239b8
Compare
8c239b8
to
1c5dc61
Compare
74a8de4
to
5c62fcd
Compare
690119d
to
7ae0c80
Compare
@bramstroker you need to take a look at the test. Looks to me like the max retries is not read from the config when running the test which is why the default of 5 is not set, and it's None. Which is why it before 7ae0c80 just ran into And yet, the measurement is failing for some reason, but I think the "None" config option needs to be fixed as well. :) |
f6d22a2
to
8398df2
Compare
8398df2
to
4ab94d7
Compare
@bramstroker after adding a workaround for the |
if not (answers.get(QUESTION_DUMMY_LOAD, False) and isinstance(self.power_meter, HassPowerMeter)): | ||
pass | ||
elif self.power_meter.autodetect_voltage_entity(answers.get(QUESTION_POWERMETER_ENTITY_ID, None)): | ||
# Autodetection succeeded | ||
pass | ||
else: | ||
# Ask voltage question if autodetection failed | ||
voltage_question = self.power_meter.get_voltage_question() | ||
if voltage_question: | ||
voltage_answer = self.ask_questions(voltage_question) | ||
answers.update(voltage_answer) | ||
else: | ||
_LOGGER.error("Voltage Meter question result was empty") | ||
exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw I just notice there is several code added now specifically for powermeters, and even more specifically for the hass powermeter only.
There is specific extension point for this designed. See line 248.
Powermeter protocol has get_questions
method where you can extend the questions with more questions in hass.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this one still needs resolving.
I have the test fixed. Could you replace the test method with this? def test_wizard(mock_config_factory) -> None: # noqa: ANN001
"""Test the CLI wizard, can we actually select certain options"""
mock_config = mock_config_factory(
set_question_defaults=False,
)
measure = _create_measure_instance(
mock_config,
console_events=event_factory(
# MEASURE_TYPE
key.ENTER,
# GENERATE_MODEL_JSON
"y",
# DUMMY_LOAD
"n",
# MODEL_NAME
"a", key.ENTER,
# MEASURE_DEVICE
"a", key.ENTER,
# COLOR_MODE
key.DOWN, key.DOWN, key.ENTER,
# GZIP
key.ENTER,
# MULTIPLE_LIGHTS
"n",
),
)
with patch("builtins.input", return_value=""):
measure.start()
assert os.path.exists(os.path.join(PROJECT_DIR, "export/dummy/brightness.csv.gz"))
assert os.path.exists(os.path.join(PROJECT_DIR, "export/dummy/model.json")) |
Also replace line 242 of |
@RubenKelevra Did you see my comments? Are you able to do final rework, than we can merge. |
@bramstroker yes, just haven't had time yet. Maybe next year 😜 Guten Rutsch :) |
No worries, take your time. Gelukkig nieuwjaar ;-) |
Co-authored-by: RubenKelevra <[email protected]>
for more information, see https://pre-commit.ci
Sure: dde0a80 |
Co-authored-by: RubenKelevra <[email protected]>
Sure: f708bb3 |
Current State
Problem Statement
Proposed Solution
This PR introduces a method to adjust dummy load power consumption measurements based on grid voltage.
Technical Explanation
Resistive loads exhibit varying power consumption depending on grid voltage. In Germany, the allowed voltage range is 230 V ± 10%, meaning a 4800 Ω resistor's power consumption can range between:
Although voltage fluctuations are typically minor in residential areas over short periods, long-term measurements (spanning several hours) are affected by slow voltage drifts. These drifts can be suddenly corrected by transformers, introducing a "sawtooth" noise in the readings.
To mitigate this, the following approach is proposed:
Rather than relying on a single measurement from the power meter, we now collect voltage and power consumption data over a 10-minute period. Using this data, we calculate the resistor's value and average it for improved accuracy.
To automate the determination of resistor stability, we leverage the same 10-minute data collection. During this period, we capture 20 consecutive 30-second averages and assess the stability by comparing the average drift of the first and second sets of ten measurements.
Advantages
Limitations
Hardware Compatibility:
Home Assistant (HA) Entity Naming:
pattern_name_power
pattern_name_voltage
.OCR Exclusion:
Accuracy of Measurements
To validate the accuracy of this approach, I conducted three test runs in my setup. In each instance, the script successfully determined whether the resistors were sufficiently preheated.
The measured resistances and corresponding power consumptions were as follows:
The variation across these runs is minimal, with the power differences staying well below the 0.1 W resolution of Gen3 Shelly sensors.
Conclusion
This PR addresses inaccuracies in resistive load power measurement by compensating for grid voltage fluctuations. Although this approach introduces some limitations, it enhances measurement reliability for supported hardware.
Due to size and complexity, I consider this a draft atm.