Skip to content

Commit 80f4f94

Browse files
TypQxQTypQxQ
committed
Refactor Modbus write logic: improve error handling and logging for register writes
1 parent 395c3fd commit 80f4f94

File tree

1 file changed

+126
-122
lines changed

1 file changed

+126
-122
lines changed

‎custom_components/sigen/modbus.py‎

Lines changed: 126 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -567,38 +567,41 @@ async def async_write_registers(
567567

568568
async with self._locks[key]:
569569
if register_type in [RegisterType.HOLDING, RegisterType.WRITE_ONLY]:
570-
# For Modbus addresses starting with 4xxxx,
571-
# some devices expect the offset (address - 40001)
572-
if address >= 40001:
573-
# Try with the offset addressing first
574-
offset_address = address - 40001
575-
_LOGGER.debug(
576-
"Writing to registers starting at %s (offset address %s) \
577-
with values %s for slave %s",
578-
address, offset_address, values, slave_id
579-
)
580-
result = await client.write_registers(
581-
address=offset_address, values=values, slave=slave_id
582-
)
583-
else:
584-
_LOGGER.debug(
585-
"Writing to registers starting at %s with values %s for slave %s",
586-
address, values, slave_id
587-
)
588-
result = await client.write_registers(
589-
address=address, values=values, slave=slave_id
590-
)
591-
570+
tried_offset = False
571+
last_error = None
572+
# Try offset addressing first if address >= 40001
573+
# if address >= 40001:
574+
# offset_address = address - 40001
575+
# _LOGGER.debug(
576+
# "Trying write_registers at offset address %s (orig %s) with values %s for slave %s",
577+
# offset_address, address, values, slave_id
578+
# )
579+
# result = await client.write_registers(
580+
# address=offset_address, values=values, slave=slave_id
581+
# )
582+
# if not result.isError():
583+
# _LOGGER.debug("Successfully wrote to registers at offset address %s", offset_address)
584+
# return
585+
# _LOGGER.debug("Offset write failed, will try original address. Error: %s", result)
586+
# last_error = result
587+
# tried_offset = True
588+
# Try original address if not tried yet or offset failed
589+
_LOGGER.debug(
590+
"Trying write_registers at original address %s with values %s for slave %s",
591+
address, values, slave_id
592+
)
593+
result = await client.write_registers(
594+
address=address, values=values, slave=slave_id
595+
)
592596
if result.isError():
593597
_LOGGER.warning("Modbus write_registers error for %s:%s@%s (address %s): %s. Marking connection as closed.", key[0], key[1], slave_id, address, result)
594598
self._connected[key] = False
595599
_LOGGER.debug("Error response from write_registers: %s", result)
596600
raise SigenergyModbusError(
597-
f"Error writing registers at address {address}: {result}"
601+
f"Error writing registers at address {address}: {result if not tried_offset else last_error}, {result}"
598602
)
599603
else:
600-
_LOGGER.debug("Successfully wrote to registers starting at address %s",
601-
address)
604+
_LOGGER.debug("Successfully wrote to registers at address %s", address)
602605
else:
603606
raise SigenergyModbusError(
604607
f"Register type {register_type} is not writable"
@@ -922,7 +925,8 @@ async def async_write_parameter(
922925
ValueError: If device_type is invalid or device_identifier is missing when required.
923926
"""
924927
if self.read_only:
925-
raise SigenergyModbusError("Cannot write parameter while in read-only mode")
928+
_LOGGER.error("Cannot write parameter while in read-only mode")
929+
return
926930

927931
slave_id: Optional[int] = None
928932
parameter_registers: Dict[str, ModbusRegisterDefinition] = {}
@@ -1005,104 +1009,104 @@ async def async_write_parameter(
10051009
# plant_info is guaranteed to be a dict here
10061010

10071011
# Special handling for plant_remote_ems_enable register
1008-
if register_name == "plant_remote_ems_enable":
1009-
_LOGGER.debug("Special handling for plant_remote_ems_enable register")
1010-
approaches = [
1011-
{"function": "write_registers", "address": register_def.address,
1012-
"values": [int(value)]},
1013-
{"function": "write_register", "address": register_def.address,
1014-
"value": int(value)},
1015-
{"function": "write_registers", "address": register_def.address - 40001,
1016-
"values": [int(value)]},
1017-
{"function": "write_register", "address": register_def.address - 40001,
1018-
"value": int(value)},
1019-
]
1020-
last_error = None
1021-
success = False
1022-
try:
1023-
async with self._locks[key]:
1024-
client = await self._get_client(device_info)
1025-
for approach in approaches:
1026-
try:
1027-
_LOGGER.debug(
1028-
"Plant write attempt: %s", approach['description'] \
1029-
if 'description' in approach \
1030-
else f"{approach['function']} @ {approach['address']}")
1031-
if approach["function"] == "write_registers":
1032-
result = await client.write_registers(
1033-
address=approach["address"],
1034-
values=approach["values"],
1035-
slave=slave_id
1036-
)
1037-
else:
1038-
result = await client.write_register(
1039-
address=approach["address"],
1040-
value=approach["value"],
1041-
slave=slave_id
1042-
)
1043-
if not result.isError():
1044-
success = True
1045-
break
1046-
last_error = result
1047-
except Exception as ex_inner:
1048-
last_error = ex_inner
1049-
if not success:
1050-
raise SigenergyModbusError("Failed plant_remote_ems_enable write after "
1051-
f"all attempts. Last error: {last_error}")
1052-
return # Success
1053-
except (ConnectionException, ModbusException, SigenergyModbusError) as ex_outer:
1054-
self._connected[key] = False
1055-
raise SigenergyModbusError(f"Error during special plant write: {ex_outer}") \
1056-
from ex_outer
1057-
except Exception as ex_outer:
1058-
raise SigenergyModbusError("Unexpected error during special plant write:"
1059-
f" {ex_outer}") from ex_outer
1012+
# if register_name == "plant_remote_ems_enable":
1013+
# _LOGGER.debug("Special handling for plant_remote_ems_enable register")
1014+
# approaches = [
1015+
# {"function": "write_registers", "address": register_def.address,
1016+
# "values": [int(value)]},
1017+
# {"function": "write_register", "address": register_def.address,
1018+
# "value": int(value)},
1019+
# {"function": "write_registers", "address": register_def.address - 40001,
1020+
# "values": [int(value)]},
1021+
# {"function": "write_register", "address": register_def.address - 40001,
1022+
# "value": int(value)},
1023+
# ]
1024+
# last_error = None
1025+
# success = False
1026+
# try:
1027+
# async with self._locks[key]:
1028+
# client = await self._get_client(device_info)
1029+
# for approach in approaches:
1030+
# try:
1031+
# _LOGGER.debug(
1032+
# "Plant write attempt: %s", approach['description'] \
1033+
# if 'description' in approach \
1034+
# else f"{approach['function']} @ {approach['address']}")
1035+
# if approach["function"] == "write_registers":
1036+
# result = await client.write_registers(
1037+
# address=approach["address"],
1038+
# values=approach["values"],
1039+
# slave=slave_id
1040+
# )
1041+
# else:
1042+
# result = await client.write_register(
1043+
# address=approach["address"],
1044+
# value=approach["value"],
1045+
# slave=slave_id
1046+
# )
1047+
# if not result.isError():
1048+
# success = True
1049+
# break
1050+
# last_error = result
1051+
# except Exception as ex_inner:
1052+
# last_error = ex_inner
1053+
# if not success:
1054+
# raise SigenergyModbusError("Failed plant_remote_ems_enable write after "
1055+
# f"all attempts. Last error: {last_error}")
1056+
# return # Success
1057+
# except (ConnectionException, ModbusException, SigenergyModbusError) as ex_outer:
1058+
# self._connected[key] = False
1059+
# raise SigenergyModbusError(f"Error during special plant write: {ex_outer}") \
1060+
# from ex_outer
1061+
# except Exception as ex_outer:
1062+
# raise SigenergyModbusError("Unexpected error during special plant write:"
1063+
# f" {ex_outer}") from ex_outer
10601064

10611065
# Special handling for U32/S32 registers
1062-
elif register_def.data_type in [DataType.U32, DataType.S32]:
1063-
_LOGGER.debug("Special handling for U32/S32 register %s", register_name)
1064-
encoded_values = self._encode_value(value=value, data_type=register_def.data_type,
1065-
gain=register_def.gain)
1066-
_LOGGER.debug("Encoded values for %s: %s", register_name, encoded_values)
1067-
approaches = [
1068-
{"address": register_def.address},
1069-
{"address": register_def.address - 40001},
1070-
{"address": register_def.address - 40000},
1071-
{"address": register_def.address % 10000},
1072-
]
1073-
last_error = None
1074-
success = False
1075-
try:
1076-
async with self._locks[key]:
1077-
client = await self._get_client(device_info)
1078-
for approach in approaches:
1079-
try:
1080-
_LOGGER.debug("Plant U32/S32 write attempt: write_registers @ %s",
1081-
approach['address'])
1082-
result = await client.write_registers(
1083-
address=approach["address"],
1084-
values=encoded_values,
1085-
slave=slave_id
1086-
)
1087-
if not result.isError():
1088-
success = True
1089-
break
1090-
last_error = result
1091-
except Exception as ex_inner:
1092-
last_error = ex_inner
1093-
if not success:
1094-
raise SigenergyModbusError(
1095-
f"Failed U32/S32 write for {register_name} after all attempts. "
1096-
f"Last error: {last_error}"
1097-
)
1098-
return # Success
1099-
except (ConnectionException, ModbusException, SigenergyModbusError) as ex_outer:
1100-
self._connected[key] = False
1101-
raise SigenergyModbusError("Error during special plant U32/S32 write:"\
1102-
f" {ex_outer}") from ex_outer
1103-
except Exception as ex_outer:
1104-
raise SigenergyModbusError("Unexpected error during special plant " \
1105-
f"U32/S32 write: {ex_outer}") from ex_outer
1066+
# if register_def.data_type in [DataType.U32, DataType.S32]:
1067+
# _LOGGER.debug("Special handling for U32/S32 register %s", register_name)
1068+
# encoded_values = self._encode_value(value=value, data_type=register_def.data_type,
1069+
# gain=register_def.gain)
1070+
# _LOGGER.debug("Encoded values for %s: %s", register_name, encoded_values)
1071+
# approaches = [
1072+
# {"address": register_def.address},
1073+
# {"address": register_def.address - 40001},
1074+
# {"address": register_def.address - 40000},
1075+
# {"address": register_def.address % 10000},
1076+
# ]
1077+
# last_error = None
1078+
# success = False
1079+
# try:
1080+
# async with self._locks[key]:
1081+
# client = await self._get_client(device_info)
1082+
# for approach in approaches:
1083+
# try:
1084+
# _LOGGER.debug("Plant U32/S32 write attempt: write_registers @ %s",
1085+
# approach['address'])
1086+
# result = await client.write_registers(
1087+
# address=approach["address"],
1088+
# values=encoded_values,
1089+
# slave=slave_id
1090+
# )
1091+
# if not result.isError():
1092+
# success = True
1093+
# break
1094+
# last_error = result
1095+
# except Exception as ex_inner:
1096+
# last_error = ex_inner
1097+
# if not success:
1098+
# raise SigenergyModbusError(
1099+
# f"Failed U32/S32 write for {register_name} after all attempts. "
1100+
# f"Last error: {last_error}"
1101+
# )
1102+
# return # Success
1103+
# except (ConnectionException, ModbusException, SigenergyModbusError) as ex_outer:
1104+
# self._connected[key] = False
1105+
# raise SigenergyModbusError("Error during special plant U32/S32 write:"\
1106+
# f" {ex_outer}") from ex_outer
1107+
# except Exception as ex_outer:
1108+
# raise SigenergyModbusError("Unexpected error during special plant " \
1109+
# f"U32/S32 write: {ex_outer}") from ex_outer
11061110

11071111
# === General Write Logic ===
11081112
# (Executes if device_type is not 'plant' or if it's a plant register

0 commit comments

Comments
 (0)