feat: Add engine dimmer with bypass

[XavierBerger]

No description provided.

[XavierBerger]

@ghisch I did update the behavior of your engine, but it should still work the same.

Can you let me know that I didn't introduce regression and tell me if from your point of view the code is ready to be merged in main?

What changed ?

• I change the bypass time by a countdown visible inside home assistant. Countdown (Bypass tempo) is set and count down when routing level reach 100% until bypass is activated
• Duration before bypass is configurable in Home Assistant and can be changed dynamically without updating router code. bypass_timer_threshold has been removed and replaced by this dynamic value.
• Regulator opening and bypass status are now sensors visible in Home Assistant and no more actuators. They reflect the actual engine behavior.
• Configuration parameter hide_regulators is no more required.

To do before merge

• @ghisch test the new code into its hardware

  engine:
    url: https://github.com/XavierBerger/Solar-Router-for-ESPHome/
    file: solar_router/engine_progressive_with_bypass.yaml
    ref: feat/engine_progressive_with_bypass
    refresh: 1d

• @XavierBerger Update the doc

[ghisch]

Hey 👋 Sorry but I'm able to see 3 regressions so far

1. Regression: When Activate Solar Routing is disabled, if Router Level is changed, the command is not sent to the dimmer nor the bypass, while Power divertion indicates that energy is diverted.
2. Regression: If you disable Activate Solar Routing while the Router Level is at 100% with bypass enabled, bypass stays enabled. Seems fixed with last PR
3. Regression: If you disable Activate Solar Routing while the Router Level is more than 0% with bypass disabled, dimmer stays open.

I see you moved all the engine logic to the energy_regulation script, but this script is only executed when Activate Solar Routing is enabled. This prevent the control of Router Level when Activate Solar Routing is disabled. This is why in my PR, I moved the logic to the component. I think the 3 regressions are linked to that.
Maybe you could compute the delta only if Activate Solar Routing is enabled, and execute the energy_regulation script for any value of solar_router (on_value.then.script.execute: energy_regulation)

I change the bypass time by a countdown visible inside home assistant. Countdown (Bypass tempo) is set and count down when routing level reach 100% until bypass is activated

Much more cleaner indeed 👍

Duration before bypass is configurable in Home Assistant and can be changed dynamically without updating router code. bypass_timer_threshold has been removed and replaced by this dynamic value.

I don't feel it's necessary because this is the kind of things you "set and forget", I don't know how much overhead it adds but I feel it's negligable so nice to have

Regulator opening and bypass status are now sensors visible in Home Assistant and no more actuators. They reflect the actual engine behavior.

Having sensors by default in HA is cleaner indeed 👍

Configuration parameter hide_regulators is no more required.

I would sill let something to be able to expose the controls of the different regulators for advanced users or to debug.

[ghisch]

Nice to have IMO:

• Precision of the Regulator Opening sensor seems ways lower than Router Level, would be great to have the same.
  

• Name of the Input Full power duration before bypass is too long, just set it as Bypass Duration or similar

• Changing Full power duration before bypass should reset Bypass tempo (to the new defined value)

[XavierBerger]

I just push some update and I think I fixed the issue you found while you were testing them...

Regression 1 is fixed : moving the router level, when the router is deactivated, is now allowed.
Regression 3 is fixed : when 100% is reached, the bypass is automatically activated.

I see you moved all the engine logic to the energy_regulation script, but this script is only executed when Activate Solar Routing is enabled. This prevent the control of Router Level when Activate Solar Routing is disabled. This is why in my PR, I moved the logic bypass logic to the component. I think the 3 regressions are linked to that.
Maybe you could compute the delta only if Activate Solar Routing is enabled, and execute the energy_regulation script for any value or solar_router.

Yes because I feel that it is easier for debugging to have the logic in only one location. And I think that with this architecture, it could be possible to move the router_level to common. When it is moved, it just had to call the regulation script and the specialized part of the engine will manage what to do.
What is your opinion about that?

I would sill let something to be able to expose the controls of the different regulators for advanced users or to debug.

In theory, everything is exposed now...

• Name of the Input Full power duration before bypass is too long, just set it as Bypass Duration or similar

• Changing Full power duration before bypass should reset Bypass tempo (to the new defined value)

Good point, I'll do it.

• Precision of the Regulator Opening sensor seems ways lower than Router Level, would be great to have the same.

Definitively... I had a look long time ago but didn't find an obvious solution (and I gave up to concentrate on other features)

[ghisch]

Regression 1 is fixed : moving the router level, when the router is deactivated, is now allowed.

Regression 1 is still present on last commit. My dimmer opening stays at 0 if I set router_level to 50 with Solar Routing disabled. (see Regression 5)

Regression 3 is fixed : when 100% is reached, the bypass is automatically activated.

Regression 3 is fixed. My relay activates when router_level is set to 100 with Solar Routing disabled

New bug detected:
4. Regression: When disabling Solar Routing while Solar Router is not at 0, the energy counter reports wrong data (79W Energy Diverted but Router Level is 0 in the ex):

5. Regression: Dimmer opening is never changed. I see in the code you never call the script to change the dimmer opening regulation_control

Yes because I feel that it is easier for debugging to have the logic in only one location. And I think that with this architecture, it could be possible to move the router_level to common. When it is moved, it just had to call the regulation script and the specialized part of the engine will manage what to do.
What is your opinion about that?

In the one hand, I agree and like the fact that all logic is defined in the script. It's cleaner, easier to understand if everything is at the same place.
But in the other hand, I'm also wondering if placing everything in the script make it more complicated.

IMO it's better to apply new logic when a state changes, not every second. Otherwise you would have to trigger you script on some value change, but your script is in mode single so can only be ran once.
🫤 But I'm not the maintainer of this code so I could understand that you favor clarity and maintenance.
As long as it works, I feel it's just implementation details.

In theory, everything is exposed now...

Values are exposed through the sensors, but the raw controls of regulators are not. For my use-case, I'd like to be able to switch on bypass relay outside of router logic, so there's no safety_limit, no energy counting, no LED feedback etc.

Definitively... I had a look long time ago but didn't find an obvious solution (and I gave up to concentrate on other features)

I don't know if it will change anything but I see there's a accuracy_decimals (ref)
I only hope the rounding happens in HA and not on the ESP, because it would means the opening of the dimmer would be way less precise (so wayyyy less precise).

[XavierBerger]

Regression 1 is still present on last commit. My dimmer opening stays at 0 if I set router_level to 50 with Solar Routing disabled. (see Regression 5)

Do you see regulator opening changing? In fact, I don't have the hardware to fully tests it... so I did consider that is regulator opening is updated it works, but it looks like it isn't. Would you help me to fix this issue and propose a code to fix it? ... I'm sorry to have broken your PR...

Regression: When disabling Solar Routing while Solar Router is not at 0, the energy counter reports wrong data (79W Energy Diverted but Router Level is 0 in the ex):

In this case, is the triac conduct when level is less than 100%? Or do we face regression 5?

But in the other hand, I'm also wondering if placing everything in the script make it more complicated.

I would say, no, it is not more complex since you can follow what happen sequentially. So yes, there is more code here, I think it will be easier to maintain. I would like also to migrate the LED management in common, like this, we don't have to manage it in engine itself. But this will come later.

Values are exposed through the sensors, but the raw controls of regulators are not. For my use-case, I'd like to be able to switch on bypass relay outside of router logic, so there's no safety_limit, no energy counting, no LED feedback etc.

OK, I understand. It makes sense to have such an option. So let's keep it. (and, later, add where it could be missing)

Regression: When disabling Solar Routing while Solar Router is not at 0, the energy counter reports wrong data (79W Energy Diverted but Router Level is 0 in the ex):

[ghisch]

Do you see regulator opening changing? In fact, I don't have the hardware to fully tests it... so I did consider that is regulator opening is updated it works, but it looks like it isn't. Would you help me to fix this issue and propose a code to fix it? ... I'm sorry to have broken your PR...

The physical TRIAC always stays disabled (at 0). The sensor regulator_opening does change.
It's ok don't worry, you did bring a lot of new features.
As mention the issue is because you never call the script regulation_control to change the physical TRIAC opening, in this PR you only change the sensor regulator_opening
Same way as you have to call relay_regulation_control to actually toggle the physical Relay.

In this case, is the triac conduct when level is less than 100%? Or do we face regression 5?

Same answer as before. Physical TRIAC opening is never (ever) changed, it is always at 0. In the screenshot you can see the regulator_opening is at 0 too in this case.

I would like also to migrate the LED management in common, like this, we don't have to manage it in engine itself. But this will come later.

Yes yes 👍

[XavierBerger]

@ghisch OK, I think I fix the behavior of the engine. Can you, please, confirm I didn't miss something?

[ghisch]

Did some quick tests and everything seems fixed indeed 👍