Skip to content

fix command timing problem #3271

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.

Sign up for GitHub

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

Open
LKuemmel wants to merge 3 commits into
base: master
Choose a base branch
from
+35 −34
Open

fix command timing problem #3271

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a2ee642
fix timing problem
LKuemmel Apr 8, 2026
e892347
review
LKuemmel Apr 9, 2026
3ea16ce
flake8
LKuemmel Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 35 additions & 34 deletions packages/helpermodules/command.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from helpermodules.pub import Pub, pub_single
from helpermodules.subdata import SubData
from helpermodules.utils.topic_parser import decode_payload, get_index
from control import bat, bridge, data, counter, counter_all, pv
from control import bat, bridge, counter, counter_all, pv
from control.ev import ev
from modules.chargepoints.internal_openwb.chargepoint_module import ChargepointModule
from modules.chargepoints.internal_openwb.config import InternalChargepointMode
Expand Down Expand Up @@ -301,10 +301,10 @@ def setup_added_chargepoint():
chargepoint_config["id"] = new_id
chargepoint_config["name"] = f'{chargepoint_config["name"]} {new_id}'
try:
evu_counter = data.data.counter_all_data.get_id_evu_counter()
data.data.counter_all_data.hierarchy_add_item_below(
evu_counter = SubData.counter_all_data.get_id_evu_counter()
SubData.counter_all_data.hierarchy_add_item_below(
new_id, ComponentType.CHARGEPOINT, evu_counter)
Pub().pub("openWB/set/counter/get/hierarchy", data.data.counter_all_data.data.get.hierarchy)
Pub().pub("openWB/set/counter/get/hierarchy", SubData.counter_all_data.data.get.hierarchy)
setup_added_chargepoint()
except (TypeError, IndexError):
if chargepoint_config["type"] == 'internal_openwb' and SubData.general_data.data.extern:
Expand All @@ -314,9 +314,9 @@ def setup_added_chargepoint():
"type": ComponentType.CHARGEPOINT.value,
"children": []
}] +
data.data.counter_all_data.data.get.hierarchy)
SubData.counter_all_data.data.get.hierarchy)
Pub().pub("openWB/set/counter/get/hierarchy", hierarchy)
data.data.counter_all_data.data.get.hierarchy = hierarchy
SubData.counter_all_data.data.get.hierarchy = hierarchy
setup_added_chargepoint()
else:
pub_user_message(payload, connection_id,
Expand Down Expand Up @@ -389,8 +389,8 @@ def removeChargepoint(self, connection_id: str, payload: dict) -> None:
remove_acl_role("chargepoint-<id>-access", cp_id)
remove_acl_role("chargepoint-<id>-write-access", cp_id)
ProcessBrokerBranch(f'chargepoint/{cp_id}/').remove_topics()
data.data.counter_all_data.hierarchy_remove_item(cp_id, ComponentType.CHARGEPOINT)
Pub().pub("openWB/set/counter/get/hierarchy", data.data.counter_all_data.data.get.hierarchy)
SubData.counter_all_data.hierarchy_remove_item(cp_id)
Pub().pub("openWB/set/counter/get/hierarchy", SubData.counter_all_data.data.get.hierarchy)
pub_user_message(payload, connection_id,
f'Ladepunkt mit ID \'{cp_id}\' gelöscht.', MessageType.SUCCESS)

Expand All @@ -400,24 +400,22 @@ def addChargepointTemplate(self, connection_id: str, payload: dict) -> None:
new_id = self.max_id_chargepoint_template + 1
# check if "payload" contains "data.copy"
if "data" in payload and "copy" in payload["data"]:
new_chargepoint_template = asdict(data.data.cp_template_data[f'cpt{payload["data"]["copy"]}'].data).copy()
new_chargepoint_template = asdict(SubData.cp_template_data[f'cpt{payload["data"]["copy"]}'].data).copy()
new_chargepoint_template["name"] = f'Kopie von {new_chargepoint_template["name"]}'
else:
new_chargepoint_template = get_chargepoint_template_default()
new_chargepoint_template["name"] = f'{new_chargepoint_template["name"]} {new_id}'
new_chargepoint_template["id"] = new_id
# if copying a template, increase id of autolock plans
if "data" in payload and "copy" in payload["data"]:
for plan in new_chargepoint_template["autolock"]["plans"]:
plan["id"] = self.max_id_autolock_plan + 1
self.max_id_autolock_plan += 1
Pub().pub("openWB/set/command/max_id/autolock_plan", self.max_id_autolock_plan)
Comment on lines +409 to +414
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The new copy-path mutates autolock plan IDs and publishes an updated max_id/autolock_plan. There’s existing pytest coverage for Command (see packages/helpermodules/command_test.py), but no test currently exercises this copy behavior; adding a unit test for addChargepointTemplate that asserts (1) copied plans get new unique IDs and (2) the published max-id matches the highest assigned plan id would help prevent regressions.

Copilot uses AI. Check for mistakes.
Pub().pub(f'openWB/set/chargepoint/template/{new_id}', new_chargepoint_template)
self.max_id_chargepoint_template = self.max_id_chargepoint_template + 1
Pub().pub("openWB/set/command/max_id/chargepoint_template",
self.max_id_chargepoint_template)
# if copying a template, copy autolock plans
if "data" in payload and "copy" in payload["data"]:
for plan in data.data.cp_template_data[f'cpt{payload["data"]["copy"]}'].data.autolock.plans:
new_plan = asdict(plan).copy()
new_plan["id"] = self.max_id_autolock_plan + 1
Pub().pub(f'openWB/set/chargepoint/template/{new_id}/autolock/{new_plan["id"]}', new_plan)
self.max_id_autolock_plan += 1
Pub().pub("openWB/set/command/max_id/autolock_plan", new_id)
pub_user_message(
payload, connection_id,
f'Neues Ladepunkt-Profil mit ID \'{new_id}\' hinzugefügt.',
Expand Down Expand Up @@ -445,7 +443,7 @@ def addAutolockPlan(self, connection_id: str, payload: dict) -> None:
"""
# check if "payload" contains "data.copy"
if "data" in payload and "copy" in payload["data"]:
for plan in data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans:
for plan in SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans:
if plan.id == payload["data"]["copy"]:
new_autolock_plan = copy.deepcopy(plan)
break
Expand All @@ -454,10 +452,10 @@ def addAutolockPlan(self, connection_id: str, payload: dict) -> None:
new_autolock_plan = AutolockPlan()
new_id = self.max_id_autolock_plan + 1
new_autolock_plan.id = new_id
data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans.append(
SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans.append(
new_autolock_plan)
Pub().pub(f'openWB/set/chargepoint/template/{payload["data"]["template"]}',
asdict(data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data))
asdict(SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data))
self.max_id_autolock_plan = new_id
Pub().pub("openWB/set/command/max_id/autolock_plan", new_id)
pub_user_message(
Expand All @@ -474,13 +472,13 @@ def removeAutolockPlan(self, connection_id: str, payload: dict) -> None:
payload, connection_id,
f'Die ID \'{payload["data"]["plan"]}\' ist größer als die '
f'maximal vergebene ID \'{self.max_id_autolock_plan}\'.', MessageType.ERROR)
for plan in data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans:
for plan in SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans:
if plan.id == payload["data"]["plan"]:
data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans.remove(plan)
SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data.autolock.plans.remove(plan)
break
Pub().pub(
f'openWB/chargepoint/template/{payload["data"]["template"]}',
dataclass_utils.asdict(data.data.cp_template_data[f'cpt{payload["data"]["template"]}'].data))
f'openWB/set/chargepoint/template/{payload["data"]["template"]}',
dataclass_utils.asdict(SubData.cp_template_data[f'cpt{payload["data"]["template"]}'].data))
pub_user_message(
payload, connection_id,
f'Plan für Sperren nach Uhrzeit mit ID \'{payload["data"]["plan"]}\' vom Profil '
Expand All @@ -494,16 +492,18 @@ def addChargeTemplate(self, connection_id: str, payload: dict) -> None:
self.max_id_charge_template = new_id
# check if "payload" contains "data.copy"
if "data" in payload and "copy" in payload["data"]:
new_charge_template = copy.deepcopy(data.data.ev_charge_template_data[f'ct{payload["data"]["copy"]}'].data)
new_charge_template = copy.deepcopy(SubData.ev_charge_template_data[f'ct{payload["data"]["copy"]}'].data)
new_charge_template.name = f'Kopie von {new_charge_template.name}'
for plan in new_charge_template.chargemode.scheduled_charging.plans:
plan.id = self.max_id_charge_template_scheduled_plan + 1
Comment on lines 492 to 498
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

When copying a charge template, the code updates plan.id values by incrementing self.max_id_charge_template_scheduled_plan / self.max_id_charge_template_time_charging_plan, but later publishes the broker topics openWB/set/command/max_id/charge_template_scheduled_plan and .../charge_template_time_charging_plan with new_id (the charge template id). This desynchronizes the max-id topics from the actual plan ids and can lead to id collisions when creating the next plan; publish the updated plan max-ids instead.

Copilot uses AI. Check for mistakes.
self.max_id_charge_template_scheduled_plan += 1
Pub().pub("openWB/set/command/max_id/charge_template_scheduled_plan", new_id)
Pub().pub("openWB/set/command/max_id/charge_template_scheduled_plan",
self.max_id_charge_template_scheduled_plan)
for plan in new_charge_template.time_charging.plans:
plan.id = self.max_id_charge_template_time_charging_plan + 1
self.max_id_charge_template_time_charging_plan += 1
Pub().pub("openWB/set/command/max_id/charge_template_time_charging_plan", new_id)
Pub().pub("openWB/set/command/max_id/charge_template_time_charging_plan",
self.max_id_charge_template_time_charging_plan)
new_charge_template = asdict(new_charge_template)
else:
new_charge_template = get_new_charge_template()
Expand Down Expand Up @@ -537,9 +537,10 @@ def _get_charge_template_by_source(self, payload: dict) -> ChargeTemplate:
Ladeprofil des Ladepunkts handelt.
"""
if payload["data"]["changed_in_theme"]:
charge_template = data.data.cp_data[f"cp{payload['data']['chargepoint']}"].data.set.charge_template
charge_template = SubData.cp_data[
f"cp{payload['data']['chargepoint']}"].chargepoint.data.set.charge_template
else:
charge_template = data.data.ev_charge_template_data[f'ct{payload["data"]["template"]}']
charge_template = SubData.ev_charge_template_data[f'ct{payload["data"]["template"]}']
return charge_template

def _pub_charge_template_to_source(self, payload: dict, charge_template: ChargeTemplate) -> None:
Expand Down Expand Up @@ -657,8 +658,8 @@ def set_default(topic: str, defaults: dict):
component_default["id"] = new_id
general_type = special_to_general_type_mapping(payload["data"]["type"])
try:
data.data.counter_all_data.hierarchy_add_item_below_evu(new_id, general_type)
Pub().pub("openWB/set/counter/get/hierarchy", data.data.counter_all_data.data.get.hierarchy)
SubData.counter_all_data.hierarchy_add_item_below_evu(new_id, general_type)
Pub().pub("openWB/set/counter/get/hierarchy", SubData.counter_all_data.data.get.hierarchy)
except ValueError:
pub_user_message(payload, connection_id, counter_all.CounterAll.MISSING_EVU_COUNTER, MessageType.ERROR)
return
Expand Down Expand Up @@ -707,7 +708,7 @@ def addEvTemplate(self, connection_id: str, payload: dict) -> None:
new_id = self.max_id_ev_template + 1
# check if "payload" contains "data.copy"
if "data" in payload and "copy" in payload["data"]:
new_ev_template = asdict(data.data.ev_template_data[f"et{payload['data']['copy']}"].data).copy()
new_ev_template = asdict(SubData.ev_template_data[f"et{payload['data']['copy']}"].data).copy()
new_ev_template["name"] = f'Kopie von {new_ev_template["name"]}'
else:
new_ev_template = dataclass_utils.asdict(EvTemplateData())
Expand Down Expand Up @@ -1225,8 +1226,8 @@ def __on_message_rm(self, client, userdata, msg):
if "openWB/system/device/" in msg.topic and "component" in msg.topic and "config" in msg.topic:
payload = decode_payload(msg.payload)
topic = type_to_topic_mapping(payload["type"])
data.data.counter_all_data.hierarchy_remove_item(payload["id"])
Pub().pub("openWB/set/counter/get/hierarchy", data.data.counter_all_data.data.get.hierarchy)
SubData.counter_all_data.hierarchy_remove_item(payload["id"])
Pub().pub("openWB/set/counter/get/hierarchy", SubData.counter_all_data.data.get.hierarchy)
client.subscribe(f'openWB/{topic}/{payload["id"]}/#', 2)
elif re.search("openWB/chargepoint/[0-9]+/config$", msg.topic) is not None:
payload = decode_payload(msg.payload)
Expand Down
Loading