Prioritätensteuerung für Fahrzeuge

[LKuemmel]

UI openWB/openwb-ui-settings#1050

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (3)

packages/control/counter_all/hierarchy.py:335

• In _add_missing_entries wird in der Warnmeldung element['id'] verwendet, obwohl hier ein fehlender Eintrag aus data_structure ergänzt wird. In diesem Codepfad ist element entweder der zuletzt iterierte Hierarchie-Eintrag (falsche ID) oder bei leerer Hierarchie sogar nicht definiert (UnboundLocalError). Stattdessen sollte die neu hinzugefügte ID (entry_num) verwendet werden.

                    pub_system_message({}, f"{component_type_to_readable_text(type_name)} mit ID {element['id']} wurde"

packages/control/counter_all/counter_all_data.py:104

• LoadmanagementPrioProtocol._remove_loadmanagement_prio_item ist typisiert als (entry: Dict) -> None, die Implementierung in LoadmanagementPrioMixin arbeitet aber mit List[Dict] und liefert ein bool zurück. Das bricht Typprüfungen und erschwert die Weiterentwicklung (z.B. bei mypy/pyright).

    def _remove_loadmanagement_prio_item(self, id: int, entry: Dict) -> None: ...

packages/control/counter_all/loadmanagement_prio.py:69

• sort_cps_by_loadmanagement_prios_nested gibt aktuell nur Ladepunkte zurück, deren EV-ID in loadmanagement_prios vorkommt. Wenn die Liste leer/unvollständig ist (z.B. beim ersten Start oder nach inkonsistenten Daten), werden verbleibende Chargepoints komplett ignoriert – die aufrufenden Algorithmen setzen dann für diese CPs keinen Strom mehr. Besser: alle nicht durch Prioritäten abgedeckten CPs am Ende als Fallback-Gruppe (in preferenced Reihenfolge) anhängen.

        sorted_cps = []
        for entry in self.data.get.loadmanagement_prios:

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

packages/helpermodules/command.py:798

• As in the add path, removing from data.data.counter_all_data operates on a replaceable algorithm snapshot. If it lags the retained MQTT state, removal can either raise IndexError for an existing vehicle or publish a stale list that discards recent priority changes. Use SubData.counter_all_data, matching the command module's hierarchy mutations.

            data.data.counter_all_data.remove_loadmanagement_prio_item(payload["data"]["id"])
            Pub().pub("openWB/set/counter/get/loadmanagement_prios",
                      data.data.counter_all_data.data.get.loadmanagement_prios)

packages/helpermodules/command.py:780

• This mutates the algorithm's periodically copied snapshot, rather than the MQTT-backed source of truth. Data.__copy_counter_data replaces this object from SubData.counter_all_data (control/data.py:350-351), and other command-side hierarchy updates use SubData; therefore an add concurrent with a newer priority edit can republish an older ordering and lose that edit. Update and publish SubData.counter_all_data instead.

This issue also appears on line 796 of the same file.

        data.data.counter_all_data.add_loadmanagement_prio_item("vehicle", new_id)
        Pub().pub("openWB/set/counter/get/loadmanagement_prios",
                  data.data.counter_all_data.data.get.loadmanagement_prios)

packages/helpermodules/update_config.py:3536

• The migration has no (ECO_CHARGING, False) bucket. Every vehicle using Eco charging with the legacy priority flag disabled therefore fails all tuple comparisons and is omitted from loadmanagement_prios; the new control loops then never allocate current to its chargepoint. Include the missing bucket and cover it in the migration test.

                       (Chargemode.ECO_CHARGING.value, True),

packages/helpermodules/setdata.py:939

• Using None accepts every payload, including malformed JSON that decode_payload returns as a string and lists with missing, duplicate, or malformed vehicle entries. This retained value is consumed directly by the new control loops, where invalid entries raise and missing vehicles are silently skipped. Validate the recursive priority schema and require each configured vehicle exactly once before publishing it.

            elif ("openWB/set/counter/get/hierarchy" in msg.topic or
                  "openWB/set/counter/get/loadmanagement_prios" in msg.topic):
                self._validate_value(msg, None)

[Copilot]

Pull request overview

Copilot reviewed 60 out of 60 changed files in this pull request and generated 2 comments.

Suppressed comments (4)

packages/helpermodules/command.py:799

• Vehicle removal also edits the algorithm snapshot instead of the authoritative SubData state. A stale snapshot can fail to find the vehicle after its broker topics have already been deleted, or publish an old ordering over a recent UI change. Remove and publish through SubData.counter_all_data, as the hierarchy commands do.
  packages/helpermodules/command.py:781
• This mutates the algorithm's periodically copied data snapshot rather than the MQTT-backed SubData.counter_all_data used by the other command handlers. Before the next copy—or after a recent UI reorder—adding a vehicle can publish a stale list and discard priorities; it also races with algorithm iteration. Update and publish the authoritative SubData object instead.

This issue also appears on line 797 of the same file.
packages/control/counter_all/counter_all_data.py:104

• The protocol declaration disagrees with the mixin implementation: the argument is a list of entries and the method returns bool. Because the mixin types self as this protocol, static analysis sees the line-17 is False check as comparing a None return and cannot verify the implementation contract.

    def _remove_loadmanagement_prio_item(self, id: int, entry: Dict) -> None: ...

packages/control/ev/charge_template.py:112

• This field is still operational, not obsolete: chargepoint.py:236 copies it into the control parameters and algorithm/filter_chargepoints.py:33 partitions every algorithm pass by it. Consequently, the new priority list cannot reorder vehicles across the old prio buckets. Either complete the algorithm migration away from this flag or document that it remains a higher-level priority.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (4)

packages/helpermodules/setdata.py:938

• The new priority payload is accepted as arbitrary JSON, while the consumer assumes a list containing valid type, id, and children fields. A payload such as null, {}, or a list missing one vehicle can therefore either raise during every control cycle or silently omit that vehicle from all four current-allocation loops (after its current was reset to None). Validate the structure, uniqueness, and completeness of vehicle IDs before republishing it.

            elif ("openWB/set/counter/get/hierarchy" in msg.topic or
                  "openWB/set/counter/get/loadmanagement_prios" in msg.topic):

packages/helpermodules/update_config_test.py:293

• This migration test invokes datastore upgrade 139, but the priority conversion is implemented by upgrade_datastore_140. Upgrade 139 never creates loadmanagement_prios, so this test fails before exercising the new migration; call 140 and expect version 140.

    uc.upgrade_datastore_139()

packages/control/ev/charge_template.py:112

• This field is still operational rather than obsolete: chargepoint.py:236 copies it into control_parameter.prio, and filter_chargepoints.py:30-36 continues partitioning every allocation pass by that value. Consequently, a vehicle migrated from the old high-priority setting is always processed before a low-priority vehicle regardless of their order in loadmanagement_prios. Stop using this legacy flag in allocation once its value has been translated into the new priority list.

    prio: bool = False  # OBSOLET seit 2.3.0

packages/control/counter_all/loadmanagement_prio.py:66

• Vehicles within a group share one allocation group, but sorting each vehicle separately makes the UI child order an undocumented secondary priority. This contradicts get_preferenced_chargepoint's tie-breaking rules: for example, the added test's grouped CPs have required currents 8 and 7, yet this code returns the 8 A CP first solely because its vehicle appears first. Collect all matching CPs in the group and call get_preferenced_chargepoint once for the combined list.

                sorted_grouped_cps = []
                for group_entry in entry["children"]:
                    grouped_cps = []
                    for cp in filtered_cps:
                        if cp.data.config.ev == group_entry["id"]:
                            grouped_cps.append(cp)
                    sorted_grouped_cps.extend(get_preferenced_chargepoint(grouped_cps))