forked from tbnobody/OpenDTU
-
Notifications
You must be signed in to change notification settings - Fork 62
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
DPL: add status, improve responsiveness #287
Merged
helgeerbe
merged 10 commits into
helgeerbe:development
from
schlimmchen:DPL-status-and-speedup
Jul 2, 2023
Merged
DPL: add status, improve responsiveness #287
helgeerbe
merged 10 commits into
helgeerbe:development
from
schlimmchen:DPL-status-and-speedup
Jul 2, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neither a PowerCommand nor an ActivePowerCommand shall be enqueued if another one is already pending.
this implementation checks all requirements for a new power limit to be calculated, one after the other. if any requirement is not met, a respective status is announced. status messages are communicated on the (serial) console. these can also be displayed easily on the web app in the future. the status texts explain clearly what the DPL is currently doing, which aids understanding how the DPL works. the status is only announced if it changes, or after a fixed interval. as each requirement is checked individually, the code readability is improved as well. previously, all the respective conditions had to be checked as well, but the statements were more complex. the DPL loop is now executed with high frequency, i.e., it does not wait for a fixed timespan to pass before checking requirements. it always aborts on the first unmet requirement. this should improve responsiveness, as the DPL checks all requirements more often. the DPL now waits for all power commands and power limit updates to complete. when that is the case, a settling time elapses. after the settling phase, the DPL waits for a new update from the inverter and from the power meter. now it can be assumed that the values are in sync. it then makes sense to calculate a new power limit immediately, which the DPL then does.
if the inverter is not configured to be sent commands to, the DPL is unable to control it, so the loop is aborted.
the DPL already took care to shut down the inverter if anything fishy was going on, mainly to make sure that the battery is not drained. however, some cases were missed: * if the configuration changed such that another inverter is now targeted, the one the DPL controlled previously was not shut down. * if the configuration changed such that another inverter (different serial number) was configured at the same index, the previous one was not shut down. this change corrects these problems by making the DPL keep a copy of the shared_ptr to the inverter. the shared_ptr is only released once the DPL shut the respective inverter down.
the unconditional solar passthrough mode, configured using MQTT, works differently than the normal mode of operation. it is also independent from the power meter reading. if this mode is active, a shortcut is taken to a function that implements the actions for this mode. this is convenient since we don't have to consider special cases in the code that handles normal mode of operation.
a new status is needed to communicate that no update was sent to the inverter because its power limit is still valid. in this case, calculating a new power limit is delayed by an exponentially increasing backoff. the maximum backoff time is ~1s, which is still plenty fast. the backoff is actually necessary for another reason: at least currently, a lot of debug messages are printed to the console. printing all that information in every DPL loop() is too much.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change focuses on two features: (1) The DPL loop is calculating a new power limit much more often, while not calculating anything while it is unreasonable to do so. (2) A DPL status is introduced, which shall make it more transparent to understand what the DPL is currently doing. The status is useful to be displayed on the web application in the future and is complementary to
getPowerLimiterState()
.More conditions are checked before calculating a new power limit than before, e.g.,
getEnableCommand()
, while the conditions that are checked should be much easiert to understand. Waiting for all radios to be idle was a very conservative condition, which is replaced by waiting for an empty command queue for the specific inverter. To implement this, a change is included that is also proposed in the upstream project at tbnobody#1093.A copy of the
shared_ptr
to the target inverter is now kept withinPowerLimiterClass
, which enabled shutting the inverter down in case the configuration is changed. This is also a small step towards support for multiple inverters, where it makes sense to keep a container of inverters (or probably inverter wrapper classes).The unconditional solar passthrough mode of operation (setup by MQTT) is now implemented in its own function, separate from the normal mode of operation. This should make sense as it is a distinct mode of operation. If the two are kept entangled, maintaining both modes might become a headache. A power meter reading is not required any more for this mode of operation.