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

[WIP] Feature Powerlimiter #108

Closed
wants to merge 23 commits into from
Closed

[WIP] Feature Powerlimiter #108

wants to merge 23 commits into from

Conversation

berni2288
Copy link

Todo: Write list of features & changes here.

Bernhard Kaszt and others added 23 commits January 3, 2023 22:39
Some values could not be deserialized and where null
…ments

* The voltage load correction factor now takes the voltage drop under load into account to not stop the producer too early already
* Fix powerlimiter interval not being set
* Set the power limit to the lowest value when the producer is stopped
* Other smaller improvements
If Victron support is enabled, the Dynamic Power Limiter will now directly pass
through all power coming from the Victron MPTT to the grid.
This is better for the battery, as it prevents the battery from being charged
unnecessarily. It also decreases unnecessary energy loses.
* Add translations for en and de for Powerlimiter & Battery admin settings
* Add SOC thresholds to Powerlimiter
@@ -35,6 +47,14 @@ void MqttSettingsClass::onMqttConnect(bool sessionPresent)
for (const auto& cb : _mqttSubscribeParser.get_callbacks()) {
mqttClient->subscribe(cb.topic.c_str(), cb.qos);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used and callbacks point to 0. I would remove all changes to this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not coming from my branch. Maybe a rebase problem? I don't have access to my IDE at the moment. When I've time, I'll do one more merge or rebase and make sure that everything is correct.

return;
}

float dcVoltage = inverter->Statistics()->getChannelFieldValue(CH1, FLD_UDC);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must be
float dcVoltage = inverter->Statistics()->getChannelFieldValue(TYPE_DC, CH1, FLD_UDC);

}

if (inverter->isProducing()) {
float acPower = inverter->Statistics()->getChannelFieldValue(CH0, FLD_PAC);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be
float acPower = inverter->Statistics()->getChannelFieldValue(TYPE_AC, CH0, FLD_PAC);

@Adminius
Copy link

May be it make sense to accept my PR #102 before this will be integrated into development branch?
than I can do my changes in development (there is not much work to do any more)
I'll try to make good working version today and give my go, OK?

@berni2288
Copy link
Author

You can just change the target branch of your PR to development and I'll also continue working in the powerlimiter branch after the first merge, so there is no need to hurry for you :)

@helgeerbe
Copy link
Collaborator

@berni2288 I merged your power_limiter branch to develop. It's working fine. So I propose you create a new feature branch from development for further power_limiter changes.

@berni2288
Copy link
Author

Thanks! I can create a branch for each pull request from now on.

@berni2288
Copy link
Author

Closing this PR since the branch already got merged to "development" manually.

@berni2288 berni2288 closed this Feb 20, 2023
Copy link

github-actions bot commented Apr 7, 2024

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants