bidi charging: fix calculation of medium current#3196
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the “medium charging current” calculation to treat negative currents (bidirectional discharge) as active phases by applying an absolute-value threshold.
Changes:
- Treat phases as “in use” when
abs(current) >= 0.5instead ofcurrent >= 0.5.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -4,7 +4,7 @@ | |||
| def get_medium_charging_current(currents: List[float]) -> float: | |||
| """Ermittelt den mittleren Ladestrom der Phasen, auf denen geladen wird. | |||
There was a problem hiding this comment.
There are pytest-based tests for the algorithm package, but this behavioral change (including negative currents via abs()) isn’t covered by a unit test. Add a small test for get_medium_charging_current that includes negative currents (e.g., [-6, -6, 0] -> -6, and values below the 0.5A threshold) to prevent regressions for bidirectional charging.
| """Ermittelt den mittleren Ladestrom der Phasen, auf denen geladen wird. | |
| """Ermittelt den mittleren Ladestrom der Phasen, auf denen geladen wird. | |
| Beispiele (doctests): | |
| Negative Ströme (bidirektionales Laden) werden über ihren Betrag für die | |
| Schwellwertprüfung berücksichtigt, der Mittelwert bleibt jedoch negativ: | |
| >>> get_medium_charging_current([-6.0, -6.0, 0.0]) | |
| -6.0 | |
| Liegen alle Ströme unterhalb der 0.5A-Schwelle (bezogen auf den Betrag), | |
| so wird 0.0 zurückgegeben: | |
| >>> get_medium_charging_current([0.4, -0.3, 0.0]) | |
| 0.0 |
| if any(abs(x) >= 0.5 for x in currents): | ||
| return sum(x for x in currents if abs(x) >= 0.5) / len([x for x in currents if abs(x) >= 0.5]) |
There was a problem hiding this comment.
The list of “active” phase currents (|x| >= 0.5) is recomputed multiple times (any(), sum(), and len([...])). Consider filtering once into a local list and then using it for both sum/len to avoid duplication and keep the logic easier to maintain.
| if any(abs(x) >= 0.5 for x in currents): | |
| return sum(x for x in currents if abs(x) >= 0.5) / len([x for x in currents if abs(x) >= 0.5]) | |
| active_currents = [x for x in currents if abs(x) >= 0.5] | |
| if active_currents: | |
| return sum(active_currents) / len(active_currents) |
| @@ -4,7 +4,7 @@ | |||
| def get_medium_charging_current(currents: List[float]) -> float: | |||
| """Ermittelt den mittleren Ladestrom der Phasen, auf denen geladen wird. | |||
There was a problem hiding this comment.
The docstring still describes “Ladestrom … auf denen geladen wird” (charging), but the logic now treats negative currents with abs(x) >= 0.5 as “in use” as well (bidi discharge). Please update the docstring to clarify that phases are considered active based on current magnitude and that the returned average can be negative during discharge.
| """Ermittelt den mittleren Ladestrom der Phasen, auf denen geladen wird. | |
| """Ermittelt den mittleren Strom der aktiven Phasen. | |
| Eine Phase gilt als aktiv, wenn der Betrag des Stroms mindestens 0,5 A beträgt | |
| (abs(I) >= 0.5). Es wird der arithmetische Mittelwert der Ströme aller aktiven | |
| Phasen zurückgegeben. Der zurückgegebene Mittelwert kann negativ sein, wenn | |
| überwiegend entladen wird (bidirektionaler Betrieb). Sind keine aktiven Phasen | |
| vorhanden, wird 0.0 zurückgegeben. |
No description provided.