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

Refactor message reception logic to further separate IM and Device layers #203

Open
mstovenour opened this issue Nov 26, 2019 · 1 comment

Comments

@mstovenour
Copy link
Contributor

mstovenour commented Nov 26, 2019

IM._process_recv_queue() needs to be refactored along with Device.receive_message() to further separate the processing at the IM layer vs. at the Device layer

  • The IM ACK transaction should never be exposed to the Device layer
  • The device layer should be invoked only for the specific message types that are device specific (e.g. MESSAGE_STANDARD_MESSAGE_RECEIVED_0X50, MESSAGE_EXTENDED_MESSAGE_RECEIVED_0X51, and MESSAGE_X10_MESSAGE_RECEIVED_0X52)
  • The IM callback list should only be used for IM messages and only scanned when receiving non-Device messages
    • To truly separate IM and Device layers, there should never be an IM level callback for a Device level message code. Likewise IM.process_recv_queue() should not need to scan the IM callbacks on device level messages
    • The Device class that underlies the IM class shares the same callback queue. When I added callbacks for handling ALL-Link Broadcasts and Cleanups they were getting called twice. So I had to add more hackish code to artificially separate the layers.
  • Could decorate the Message class with isImMessage and isDeviceMessage properties to make case statements simple. Default is im=true and device=false. The device specific classes that inherit from Message could set these properties (e.g. standardReceive and extendedReceive would set im==false and device=true)

I personally found this one of the most confusing aspects of the library because the IM and Device call back lists have the same name and are both invoked on every message. It took me a while to keep my head straight on which list was influencing each received message. Likewise the Device layer tries to process all the IM messages if they contain a valid "address" property. These two layers are doing some relatively hackish things to keep from processing each other's messages like looking for the presence of flags (hasattr(msg, 'flags')).

I am more than willing to submit a pull request for this but I do not have enough devices to do the extensive amount of testing that will be required to confirm I didn't break anything. If someone will help me with testing, I'll get to working on a pull request.

@teharris1
Copy link
Collaborator

@mstovenour I have started a new library that has much better layer separation. It is called pyinsteon. You can find it on github. I would be very interested in feedback as I am at release candidate 3 right now.

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

No branches or pull requests

2 participants