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

Don't let refresh requests pile up while not awake #521

Merged
merged 2 commits into from
Sep 7, 2023

Conversation

tstabrawa
Copy link
Contributor

This change fixes a bug whereby identical refresh requests may pile up in a non-awake device's send queue over time. For example, I had an automation running at 15-minute intervals that would perform a refresh-all command. Several days later, when I performed an "awake" command against a mini-remote device (2342-232), Insteon-MQTT tried to send several hundred refresh commands to the mini-remote.

This change avoids queuing more than one refresh command by replacing identical queued refresh commands with newly-received refresh commands (under the assumption that it might be more convenient to the user if the new command is still waiting for a response at the time the device is woken up).

Confirmed no new flake8 errors, pylint errors, or code coverage gaps. All tests passing. Also, confirmed new test (test_queued_refresh_replace) fails prior to applying fix:

================================================================================ FAILURES ================================================================================
______________________________________________________________ Test_Base_Config.test_queued_refresh_replace ______________________________________________________________

self = <test_BatterySensorDev.Test_Base_Config object at 0x7f209f3739a0>, test_device = <insteon_mqtt.device.BatterySensor.BatterySensor object at 0x7f209ed35000>

    def test_queued_refresh_replace(self, test_device):
        # Queue multiple refresh commands
        msg1 = Msg.OutStandard.direct(test_device.addr, 0x19, 0x00)
        msg2 = Msg.OutStandard.direct(test_device.addr, 0x19, 0x00)
        msg3 = Msg.OutStandard.direct(test_device.addr, 0x19, 0x00)
        msg_handler1 = IM.handler.StandardCmd(msg1, None, None)
        msg_handler2 = IM.handler.StandardCmd(msg2, None, None)
        msg_handler3 = IM.handler.StandardCmd(msg3, None, None)
        test_device.send(msg1, msg_handler1)
        test_device.send(msg2, msg_handler2)
        test_device.send(msg3, msg_handler3)
        # Confirm that only the last command is queued
>       assert len(test_device._send_queue) == 1
E       assert 3 == 1
E        +  where 3 = len([[<insteon_mqtt.message.OutStandard.OutStandard object at 0x7f209ed34700>, <insteon_mqtt.handler.StandardCmd.StandardC...ndard object at 0x7f209ed37fa0>, <insteon_mqtt.handler.StandardCmd.StandardCmd object at 0x7f209ed36f20>, False, None]])
E        +    where [[<insteon_mqtt.message.OutStandard.OutStandard object at 0x7f209ed34700>, <insteon_mqtt.handler.StandardCmd.StandardC...ndard object at 0x7f209ed37fa0>, <insteon_mqtt.handler.StandardCmd.StandardCmd object at 0x7f209ed36f20>, False, None]] = <insteon_mqtt.device.BatterySensor.BatterySensor object at 0x7f209ed35000>._send_queue

tests/device/test_BatterySensorDev.py:149: AssertionError
======================================================================== short test summary info =========================================================================
FAILED tests/device/test_BatterySensorDev.py::Test_Base_Config::test_queued_refresh_replace - assert 3 == 1
===================================================================== 1 failed, 706 passed in 10.71s =====================================================================

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.
  • Code documentation was added where necessary - N/A

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated - N/A

@krkeegan krkeegan merged commit f11f091 into TD22057:dev Sep 7, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants