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

DPL: Honor battery-provided discharge power limit (including UI adjustments) #1245

Conversation

AndreasBoehm
Copy link

@AndreasBoehm AndreasBoehm commented Sep 10, 2024

Based on the great work from @ranma!
Base: #1198

Additions

  • add UI to configure limit manually
  • add toggle to enable/disable the use of the battery (bms) provided limit
  • add support for discharge current limit for MQTT battery provider
  • show live view for MQTT battery when discharge current limit is present
MQTT Battery Provider Other Battery Providers MQTT Battery Live View
Screenshot 2024-09-10 at 19 22 55 Screenshot 2024-09-10 at 19 23 18 Screenshot 2024-09-10 at 19 28 43

Add discharge current limit as a input field with the option to use the battery reported limit as well. (All battery providers including mqtt)
@spcqike
Copy link

spcqike commented Sep 10, 2024

One question after a first glance:
How does the manual limit fit to the BMS value? The hint says it’s a fallback. So I guess the BMS value has greater priority, even I f it’s higher than the manual value?
so a manual limit of like 10A won’t work, if the BMS reports 25A?

@AndreasBoehm
Copy link
Author

Good catch @spcqike, i need to update the hint text.

Whatever limit is lower will be used

min(bmsLimit, userLimit)

@AndreasBoehm
Copy link
Author

Screenshots updated

@AndreasBoehm
Copy link
Author

We will now also show a live view for MQTT batteries when the discharge current limit is present.

Copy link
Collaborator

@schlimmchen schlimmchen left a comment

Choose a reason for hiding this comment

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

Some nitpicking to annoy you 😉

include/BatteryStats.h Outdated Show resolved Hide resolved
src/BatteryStats.cpp Show resolved Hide resolved
src/Configuration.cpp Outdated Show resolved Hide resolved
src/WebApi_battery.cpp Outdated Show resolved Hide resolved
src/WebApi_battery.cpp Outdated Show resolved Hide resolved
webapp/src/types/BatteryConfig.ts Outdated Show resolved Hide resolved
@schlimmchen schlimmchen force-pushed the dpl-discharge-power-limiter-mine branch from 3566f28 to 599b460 Compare September 13, 2024 18:42
@schlimmchen
Copy link
Collaborator

Github did not manage to detect that the first commit is merely already on development. I had to rebase manually. This set me as the committer, which I don't like and wasn't my intention. After a quick Internet search it seems this is tedious to avoid...

@AndreasBoehm
Copy link
Author

Dont worry about having my name on a commit.. its alright.

after a rebase we will be both mentioned in the commit, thats fine for me

@schlimmchen schlimmchen merged commit a6e7007 into helgeerbe:development Sep 13, 2024
8 checks passed
@schlimmchen
Copy link
Collaborator

image

Interesting. "Squash and Merge" cleaned it up the way I wanted it to. Guess "authored by and committed by" is only ever a thing when pushing from the CLI? Don't know for sure. Glad that this turned out well 😉

its alright.

Thanks!

@AndreasBoehm AndreasBoehm deleted the dpl-discharge-power-limiter-mine branch September 13, 2024 19:40
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

Successfully merging this pull request may close these issues.

3 participants