Skip to content

Repeating the EVSE readout process in case of a timeout/error to prevent unnecessary log entries #3175

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

Merged
LKuemmel merged 1 commit into from
Mar 6, 2026
Merged

Repeating the EVSE readout process in case of a timeout/error to prevent unnecessary log entries #3175

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 17 additions & 6 deletions packages/modules/common/hardware_check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import time
import pymodbus
from typing import Any, Optional, Protocol, Tuple, Union

Expand Down Expand Up @@ -80,12 +81,22 @@ def handle_exception(self: ClientHandlerProtocol, exception: Exception):

def request_and_check_hardware(self: ClientHandlerProtocol,
fault_state: FaultState) -> Tuple[EvseState, CounterState]:
try:
with self.client:
evse_state = self.evse_client.get_evse_state()
evse_check_passed = True
except Exception as e:
evse_check_passed = self.handle_exception(e)
evse_check_passed = False
evse_state: EvseState
# 2x Retry bei EVSE-Auslesen vor dem Absetzen einer Fehlermeldung
for attempt in (1, 2, 3):
try:
with self.client:
evse_state = self.evse_client.get_evse_state()
evse_check_passed = True
break
except Exception as e:
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The retry loop catches and retries on any Exception. This will also retry non-transient errors (e.g., ValueError from Evse.get_plug_charge_state() when the EVSE reports an unknown state), delaying a real fault and potentially masking programming/configuration issues. Consider restricting retries to known transient Modbus/IO failures (e.g., pymodbus connection/io exceptions, or exceptions whose __cause__ is one of those), and fail fast for semantic errors like ValueError.

Suggested change
except Exception as e:
except (pymodbus.exceptions.ModbusIOException,
pymodbus.exceptions.ConnectionException) as e:

Copilot uses AI. Check for mistakes.
evse_check_passed = self.handle_exception(e)
# Wenn nicht "handled" -> maximal zwei Wiederholungen
if attempt < 3 and evse_check_passed is False:
time.sleep(0.3)
continue
Comment on lines +86 to +98
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The retry configuration uses magic values (for attempt in (1, 2, 3) and time.sleep(0.3)). To make future tuning easier and avoid scattering policy in-line, consider extracting these into named constants (e.g., max_attempts / retry_delay_seconds) near the other module-level constants.

Copilot uses AI. Check for mistakes.
break
Comment on lines +87 to +99
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

with self.client: is executed inside the retry loop, so a failing EVSE read can open/close (and potentially reconnect) up to 3 times. Previously this block ran once per hardware check. Consider entering the client context once (outside the loop) and retrying only the get_evse_state() call to avoid extra connection churn and reduce load/latency.

Suggested change
for attempt in (1, 2, 3):
try:
with self.client:
evse_state = self.evse_client.get_evse_state()
evse_check_passed = True
break
except Exception as e:
evse_check_passed = self.handle_exception(e)
# Wenn nicht "handled" -> maximal zwei Wiederholungen
if attempt < 3 and evse_check_passed is False:
time.sleep(0.3)
continue
break
with self.client:
for attempt in (1, 2, 3):
try:
evse_state = self.evse_client.get_evse_state()
evse_check_passed = True
break
except Exception as e:
evse_check_passed = self.handle_exception(e)
# Wenn nicht "handled" -> maximal zwei Wiederholungen
if attempt < 3 and evse_check_passed is False:
time.sleep(0.3)
continue
break

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +99
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

This change makes request_and_check_hardware() call get_evse_state() up to 3 times. Existing unit tests/mocks that provide a single-item side_effect list for get_evse_state will now be exhausted (raising StopIteration) and may fail for the wrong reason. Update/add tests to cover: (1) success after 1–2 failures and (2) failure after 3 attempts, ideally also asserting the number of calls and that the delay is applied (with time.sleep mocked).

Copilot uses AI. Check for mistakes.
meter_check_passed, meter_error_msg, counter_state = self.check_meter()
if meter_check_passed is False and evse_check_passed is False:
if isinstance(self.client, ModbusTcpClient_):
Expand Down