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.

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

Generator start/stop: add new condition: "Stop generator if tank level is below x%" #1420

Open
ReinvdZee opened this issue Jan 31, 2025 · 11 comments
Assignees
Milestone

Comments

@ReinvdZee
Copy link
Collaborator

Add a new condition to generator start/stop to stop the generator if the tank level is below a certain percentage. The user should be able to select which tank sensor to be used for evaluating the condition. Some gensets have a tank sensor built-in which will show up as dedicated tank sensor on the UI. Other gensets don't have it but users may install one and hook it up to one of the GX's dedicated tank inputs.

@ReinvdZee ReinvdZee added this to the v3.60 milestone Jan 31, 2025
@ReinvdZee ReinvdZee self-assigned this Jan 31, 2025
@ReinvdZee
Copy link
Collaborator Author

ReinvdZee commented Feb 5, 2025

In dbus-generator, the generator is started when one or more of the enabled conditions evaluates to true, and is stopped when all of the enabled conditions evaluate to false. Since it does not make sense to start the generator based on a fuel level, this condition can only be used to stop the generator while it was already running by another condition or by a manual start. This means that this condition needs to override the other conditions in order to stop the generator when the fuel level is below some level.

So as soon as the tank level condition evaluates to true, all other enabled conditions are overridden and the generator is stopped. When the tank evaluates to false again, the other conditions are not overridden anymore. This will start the generator again if another condition is still active and thus no longer overridden.

@philipptrenz Must the manual start also be overridden by this condition? I.e., stop the generator automatically based on the tank level condition, even if it was manually started?

GUI-v2
I added the "Tank level" condition to the UI:
Image
The start value is omitted, as well as the timer for the stop value (stop immediately when the fuel level is reached). Different values for quiet hours is also not supported.

I also added a dropdown menu to select the tank sensor to use for this condition. Currently, this does not check the liquid type of the sensor, any liquid type is accepted.

@philipptrenz
Copy link
Collaborator

Hey @ReinvdZee, great work! The UI looks nice and not filtering for the liquid type is just fine, no need to overcomplicate it.

Must the manual start also be overridden by this condition? I.e., stop the generator automatically based on the tank level condition, even if it was manually started?

Personally, I would say it should also stop the generator during a manual run, as it's purpose is to prevent the generator to run dry.

Now, after reading your description, I wonder if it is a bit ambiguous having a condition, that overrules all other conditions without any feedback to the user. Maybe we can add an option to raise a warning in case the condition is triggered?

@ReinvdZee
Copy link
Collaborator Author

@philipptrenz

Personally, I would say it should also stop the generator during a manual run, as it's purpose is to prevent the generator to run dry.

Agreed, then lets have this condition stop the generator in all cases.

Maybe we can add an option to raise a warning in case the condition is triggered?

I like that; I can add an option to trigger a warning when it happens. Or should we just always raise a warning? Typically, it won't happen that often that the generator is stopped due to this condition.

@philipptrenz
Copy link
Collaborator

@ReinvdZee Nice!

I like that; I can add an option to trigger a warning when it happens. Or should we just always raise a warning?

If it does not add too much complexity and work for you, I would provide the user with an option to disable the warning, being enabled by default.

@ReinvdZee
Copy link
Collaborator Author

@philipptrenz

I added the setting com.victronenergy.settings/Settings/Generatorx/TankLevel/WarningEnabled and the alarm path com.victronenergy.generator.startstopx/Alarms/StoppedByTankLevelCondition. UI now looks like this:

Image

And the warning:

Image

Any comments on the UI / text?
Do we need the actual tank level value in the warning (something like "Generator stopped because tank level at 9%")? Then I'd need to add a path /TankLevel to dbus-generator which venus-platform can read when the warning is triggered.

@philipptrenz
Copy link
Collaborator

philipptrenz commented Feb 6, 2025

@ReinvdZee

I think we can omit the "by this condition" in the "Trigger warning..." text, should be clear in the context of this menu entry and might prevent space issues on smaller screens (in case this is not already the 5" version). Regarding reporting the tank level in the warning, I think it is not necessary. The warning should be indication enough to look it up manually.

ReinvdZee added a commit to victronenergy/dbus_generator that referenced this issue Feb 6, 2025
Add new condition to stop the generator when the tank level is below a certain value
Overrides all conditions, periodic runs and manual start.

victronenergy/venus#1420
ReinvdZee added a commit to victronenergy/venus-platform that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
@ReinvdZee
Copy link
Collaborator Author

@philipptrenz

I think we can omit the "by this condition"

Agreed, I removed it:
Image

I pushed a draft implementation. Since this condition overrides all other conditions, periodic run and manual start, I introduced a new state: "Stopped by tank level" (open for name improvements, maybe something like "Low fuel level" is better).

Text for the new state is also added to gui-v2:
Image

Whenever the tank level condition evaluates to true, dbus-generator will enter that state, stop the generator if its running and won't evaluate other conditions or accept manual starts and periodic runs. The cool-down setting is respected, so the generator cools down properly when stopped by this condition. I assume the generator is allowed to run for a minute more when this condition is triggered.

When the generator is stopped due to this condition, the manual start path is also cleared to prevent the generator from starting again when the tank level rises. Also, writing 1 to /ManualStart is prohibited as long as the condition is active.

Some loose ends we may need to address:

  1. The GUI does not behave differently when the generator is prohibited to start due to this condition (other than what's shown in the state field). So the user can press "manual start" and the dialog opens. Pressing "Start now" does not do anything and the dialog stays open (because it fails to start while the GUI expects it to). Maybe we can grey out the manual start button in this case.
  2. If a different start/stop condition evaluates to true when the tank level condition is resolved (i.e., tank is filled), the generator will be started right away. So e.g., say a generator was started due to inverter overload and the fuel level dropped too low, causing the generator to be stopped again. If the fuel tank is now filled and the tank sensor reflects that, the generator will be started immediately if the inverter is still overloaded. Not sure if this is an issue, but we can automatically disable autostart when the tank level condition stops the generator, to prevent the generator to be started right away after filling the tank. The user would then need to re-enable autostart when the tank level condition is resolved.
  3. This condition is, like the others, only evaluated when autostart is enabled. If we decide to disable autostart (point 2) when the condition triggers, this may need to change.
  4. The setting for the selected tank service defaults to nothing to force the user to explicitly select one. There is no "Automatic" option. I think this is fine.

ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
ReinvdZee added a commit to victronenergy/gui-v2 that referenced this issue Feb 6, 2025
@philipptrenz
Copy link
Collaborator

philipptrenz commented Feb 6, 2025

@ReinvdZee I like the new "Stopped by tank level" state and the UI screenshots!

I assume the generator is allowed to run for a minute more when this condition is triggered.

Yes, I also think that's fine. Of course, it would be nice if this behaviour is communicated to the user, so that the user can plan a certain reserve for the cool-down. But not strictly necessary in my opinion.

To 1.: Yes, greying out the manual start button sounds good

To 2. and 3.: I see the point, but disabling autostart automatically is not the nicest from a user experience perspective. It would need to be explained very well. I might have an idea to circumvent that a bit:

How about defining a second threshold "Prevent start until tank level is above"? That would
a) help with tank sensors whose values jump up and down somewhat (I have already seen that in the field)
b) make clear, that the generator will be "locked" to a certain point and "released" after that, following its configured start/stop conditions again

To 4.: Yes, I also think that's fine.

@ReinvdZee
Copy link
Collaborator Author

ReinvdZee commented Feb 10, 2025

@philipptrenz

How about defining a second threshold "Prevent start until tank level is above"?

I agree that's a nice solution for this. I've added the setting to the GUI:

Image

The generator will stop when the stop value is reached and can only start again is the level is above the "Prevent start until" value. This based on the non-persistent state "Stopped by tank level", so if the service is restarted while the generator is prevented from starts, it will be able to run again until the stop value is reached.

@philipptrenz
Copy link
Collaborator

@ReinvdZee Nice! Also disabling this condition is an option to circumvent the start prevention, right?

@ReinvdZee
Copy link
Collaborator Author

@philipptrenz Yes indeed. If the value is higher than stop, but lower than prevenstart, disabling and reenabling the condition will allow starts again.

ReinvdZee added a commit to victronenergy/dbus_generator that referenced this issue Feb 10, 2025
When the generator was stopped due to the tank level condition,
prevent starting it until the tank level reaches the `/PreventStartValue`.
This is to handle tank sensors with fluctuating values

victronenergy/venus#1420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants