Skip to content

Loadpoint: rename chargeCurrent to offeredCurrent (BC) #20457

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

Merged
merged 3 commits into from
Apr 9, 2025

Conversation

premultiply
Copy link
Member

@premultiply premultiply commented Apr 6, 2025

The name chargeCurrent at the loadpoint is confusing because, unlike chargeCurrents and chargePower, it is not a measured value but the setpoint value that is sent to the charger.

This PR therefore changes its name to offeredCurrent.

For the API exports, the old export name will also remain for the time being, but will be considered deprecated.

/cc @andig @naltatis @mfuchs1984

TODO

  • rename in UI/App (@naltatis)
  • align log messages with new naming @premultiply
  • OPTION: redefine and publish lp.chargeCurrent as "max(lp.chargeCurrents[])": maximum measured charge current for better API compatibility (non-BC)

Out of scope

@premultiply premultiply marked this pull request as ready for review April 6, 2025 21:24
@premultiply premultiply added the infrastructure Basic functionality label Apr 6, 2025
@andig
Copy link
Member

andig commented Apr 7, 2025

Wenn wir das machen dann sollte es komplett sein. Keine doppelten Namen.

@andig andig marked this pull request as draft April 7, 2025 04:47
@naltatis
Copy link
Member

naltatis commented Apr 7, 2025

Der UI Change ist überschaubar, soll ich den hier mit rein machen damit wirs sauber haben?

@andig
Copy link
Member

andig commented Apr 7, 2025

Ja bitte- ganz oder gar nicht.

@andig
Copy link
Member

andig commented Apr 7, 2025

Auch das Logging muss mit angepasst werden.

@andig
Copy link
Member

andig commented Apr 7, 2025

@premultiply wozu brauchen wir

  1. redefine and publish lp.chargeCurrent as "max(lp.chargeCurrents[])": maximum measured charge current

@andig andig changed the title rename chargeCurrent to offeredCurrent at loadpoint Loadpoint: rename chargeCurrent to offeredCurrent (BC) Apr 7, 2025
@naltatis
Copy link
Member

naltatis commented Apr 7, 2025

fyi @marq24

@naltatis
Copy link
Member

naltatis commented Apr 7, 2025

UI ist angepasst und das publishing des alten Wert (chargeCurrent) entfernt (BC)

@marq24
Copy link
Contributor

marq24 commented Apr 7, 2025

so chargeCurrent will be renamed, but the array chargeCurrents will stay... this will generate some issues in HA... but I must check, if and how this can be handled....

I just 'invent' key_alias - so now I can get the value by two different keys - in HA the name will be still the old one...

marq24 added a commit to marq24/ha-evcc that referenced this pull request Apr 7, 2025
…add the new name as alias...

evcc-io/evcc#20457
chargeCurrent -> offeredCurrent
@premultiply
Copy link
Member Author

The option is to remap the name chargeCurrent to the content of max(chargeCurrents[]) and keep publishing it in addition and next to the new published value offeredCurrent.

So the name would stay but the meaning would slightly differ from before. I think this would not break anything in most cases as it would show correctly what was just suggested before.

@naltatis
Copy link
Member

naltatis commented Apr 8, 2025

So the name would stay but the meaning would slightly differ from before.

I’d rather remove the existing key and accept the breaking change.

@marq24
Copy link
Contributor

marq24 commented Apr 8, 2025

So the name would stay but the meaning would slightly differ from before.

I’d rather remove the existing key and accept the breaking change.

I am totally fine with that: your project -> your rules :-D

@premultiply premultiply marked this pull request as ready for review April 8, 2025 19:59
@premultiply
Copy link
Member Author

LGTM?

@naltatis naltatis merged commit 598ccca into master Apr 9, 2025
6 checks passed
@naltatis naltatis deleted the core/offeredCurrent branch April 9, 2025 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Basic functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants