Whole file
marcelblijleven/goodwe
The author described this change as “Fix multiple inverters communication”. It counts as a record because the checks below fail on the code as it stood at 7fe0fb58e and pass on 4867b4acd, with nothing else changed between the two runs.
Projectmarcelblijleven/goodwe
Fix saved2022-01-08
Sharing licenceMIT · LICENSE
Change size+12 −11
What the code was meant to do, written into the code itself as a save note
Fix multiple inverters communication
The change
| 16 | 16 | class UdpInverterProtocol(asyncio.DatagramProtocol): | |
| 17 | 17 | def __init__( | |
| 18 | 18 | self, | |
| 19 | + | response_future: Future, | |
| 19 | 20 | command: ProtocolCommand, | |
| 20 | 21 | timeout: int, | |
| 21 | 22 | retries: int | |
| 22 | 23 | ): | |
| 23 | 24 | super().__init__() | |
| 25 | + | self.response_future: Future = response_future | |
| 24 | 26 | self.command: ProtocolCommand = command | |
| 25 | 27 | self._transport: asyncio.transports.DatagramTransport | None = None | |
| 26 | 28 | self._retry_timeout: int = timeout | |
| ⋯ | |||
| 37 | 39 | if exc is not None: | |
| 38 | 40 | logger.debug(f'Socket closed with error: {exc}') | |
| 39 | 41 | # Cancel Future on connection lost | |
| 40 | - | if not self.command.response_future.done(): | |
| 41 | - | self.command.response_future.cancel() | |
| 42 | + | if not self.response_future.done(): | |
| 43 | + | self.response_future.cancel() | |
| 42 | 44 | ||
| 43 | 45 | def datagram_received(self, data: bytes, addr: Tuple[str, int]) -> None: | |
| 44 | 46 | """On datagram received""" | |
| 45 | 47 | if self.command.validator(data): | |
| 46 | 48 | logger.debug(f'Received: {data.hex()}') | |
| 47 | - | self.command.response_future.set_result(data) | |
| 49 | + | self.response_future.set_result(data) | |
| 48 | 50 | else: | |
| 49 | 51 | logger.debug(f'Received invalid response: {data.hex()}') | |
| 50 | 52 | self._retries += 1 | |
| ⋯ | |||
| 53 | 55 | def error_received(self, exc: Exception) -> None: | |
| 54 | 56 | """On error received""" | |
| 55 | 57 | logger.debug(f'Received error: {exc}') | |
| 56 | - | self.command.response_future.set_exception(exc) | |
| 58 | + | self.response_future.set_exception(exc) | |
| 57 | 59 | ||
| 58 | 60 | def _send_request(self) -> None: | |
| 59 | 61 | """Send message via transport""" | |
| ⋯ | |||
| 64 | 66 | ||
| 65 | 67 | def _retry_mechanism(self) -> None: | |
| 66 | 68 | """Retry mechanism to prevent hanging transport""" | |
| 67 | - | if self.command.response_future.done(): | |
| 69 | + | if self.response_future.done(): | |
| 68 | 70 | self._transport.close() | |
| 69 | 71 | elif self._retries < self._max_retries: | |
| 70 | 72 | logger.debug('Failed to receive response to %s in time (%ds).', self.command, self._retry_timeout) | |
| ⋯ | |||
| 72 | 74 | self._send_request() | |
| 73 | 75 | else: | |
| 74 | 76 | logger.debug('Max number of retries (%d) reached, request %s failed.', self._max_retries, self.command) | |
| 75 | - | self.command.response_future.set_exception(MaxRetriesException) | |
| 77 | + | self.response_future.set_exception(MaxRetriesException) | |
| 76 | 78 | ||
| 77 | 79 | ||
| 78 | 80 | class ProtocolCommand: | |
| ⋯ | |||
| 81 | 83 | def __init__(self, request: bytes, validator: Callable[[bytes], bool]): | |
| 82 | 84 | self.request: bytes = request | |
| 83 | 85 | self.validator: Callable[[bytes], bool] = validator | |
| 84 | - | self.response_future: Future | None = None | |
| 85 | 86 | ||
| 86 | 87 | def __repr__(self): | |
| 87 | 88 | return self.request.hex() | |
| ⋯ | |||
| 95 | 96 | Return raw response data | |
| 96 | 97 | """ | |
| 97 | 98 | loop = asyncio.get_running_loop() | |
| 98 | - | self.response_future = loop.create_future() | |
| 99 | + | response_future = loop.create_future() | |
| 99 | 100 | transport, _ = await loop.create_datagram_endpoint( | |
| 100 | - | lambda: UdpInverterProtocol(self, timeout, retries), | |
| 101 | + | lambda: UdpInverterProtocol(response_future, self, timeout, retries), | |
| 101 | 102 | remote_addr=(host, GOODWE_UDP_PORT), | |
| 102 | 103 | ) | |
| 103 | 104 | try: | |
| 104 | - | await self.response_future | |
| 105 | - | result = self.response_future.result() | |
| 105 | + | await response_future | |
| 106 | + | result = response_future.result() | |
| 106 | 107 | if result is not None: | |
| 107 | 108 | return result | |
| 108 | 109 | else: | |
The check that tells the two apart
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_connection_lost
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_connection_lost_not_done
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_connection_made
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_datagram_received
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_error_received
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_modbus_read_command
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_modbus_write_command
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_retry_mechanism
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_retry_mechanism_max_retries
fail→pass·tests/test_protocol.py::TestUDPClientProtocol::test_retry_mechanism_two_retries
Check file tests/test_protocol.py, taken without changes from the fix and copied onto the older code, so the exact same check runs against both versions.
Origin and history
The code before it7fe0fb58e6581dc682e1104f0beed8870d0caccd
Broken version dated2022-01-08
Modulegoodwe.protocol
Units changedProtocolCommand, UdpInverterProtocol
Fingerprint14d538f124e7fa65
Checked2026-08-18 by goldset/0.1
Every field above is generated by our program. None of it is written by hand.
Other bugs found in marcelblijleven/goodwe
- 2026-03-29Fix duplicate sensor registrations
- 2024-05-14Fix decoding 0 energy values
- 2024-05-12Fix writing single byte settings on ES inverters
- 2024-05-08Fix setup of BT inverters
- 2024-04-08Fix decoding of voltage values
- 2024-01-09Fix deconding 0x55 as not-set value of EcoMode