Skip to content
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.

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

Calling an API method before any packets have arrives results in an exception #231

Open
sdwilsh opened this issue Dec 2, 2023 · 3 comments

Comments

@sdwilsh
Copy link
Owner

sdwilsh commented Dec 2, 2023

It used to be the case that we could call something like synchronize_time() immediately, and everything was okay. However, that no longer is the case with the addition of ECM support:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/src/brultech_serial2mqtt/brultech_serial2mqtt/__main__.py", line 9, in <module>
    loop.run_until_complete(bs2m.start())
  File "/usr/lib/python3.11/asyncio/base_events.py", line 653, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/src/brultech_serial2mqtt/brultech_serial2mqtt/__init__.py", line 54, in start
    await self._setup_gem(device_connection)
  File "/src/brultech_serial2mqtt/brultech_serial2mqtt/__init__.py", line 119, in _setup_gem
    await connection.synchronize_time()
  File "/usr/local/lib/python3.11/dist-packages/aiobrultech_serial/__init__.py", line 127, in synchronize_time
    success = await api.synchronize_time(self._protocol)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/siobrultech_protocols/gem/api.py", line 213, in synchronize_time
    return await set_date_and_time(protocol, time, serial_number, timeout)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/siobrultech_protocols/gem/api.py", line 163, in set_date_and_time
    async with call_api(SET_DATE_AND_TIME, protocol, serial_number, timeout) as f:
  File "/usr/lib/python3.11/contextlib.py", line 204, in __aenter__
    return await anext(self.gen)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/siobrultech_protocols/gem/api.py", line 38, in call_api
    delay = protocol.begin_api_request()
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/siobrultech_protocols/gem/protocol.py", line 449, in begin_api_request
    if self.api_type == ApiType.GEM and self.send_packet_delay:
       ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/siobrultech_protocols/gem/protocol.py", line 388, in api_type
    assert result

I don't see a great way to fix this, so I'm going to work-around it, but I wanted to file the issue in case @jkeljo had any ideas.

@jkeljo
Copy link
Collaborator

jkeljo commented Dec 2, 2023

Yeah, tricky. If you know what you're connecting to you can pre-configure the protocol for that API type.

I played around just now with sending an ECM API request to my GEM (ignored; client eventually timed out), and sending a GEM API request to my ECM (that mostly also just ignores it until the client times out, though one time I did see the client get confused and try to parse an incoming binary packet as utf-8). So that's not ideal, but I could imagine writing a detection routine based on the idea that a real API response comes in very quickly (like a timeout of 200ms would likely be plenty). It would probably get it wrong (and decide that it is neither a GEM nor an ECM) every once in a blue moon when the system's under load or something and it isn't able to process the response in time, but that's not too big a deal because you can just fall back to waiting for the first packet to figure it out.

@jkeljo
Copy link
Collaborator

jkeljo commented Dec 2, 2023

We should probably raise an actual exception here rather than assert, just so that it's clearer what is going on.

@sdwilsh
Copy link
Owner Author

sdwilsh commented Dec 2, 2023

We should probably raise an actual exception here rather than assert, just so that it's clearer what is going on.

Yeah, it took me a bit to figure out what went wrong. I'm going to work around this and just require it to be set in the config for brultech-serial2mqtt at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants