python icon indicating copy to clipboard operation
python copied to clipboard

Make Serial and TCP interfaces more closely respect connectNow when false

Open Travis-L-R opened this issue 1 month ago • 2 comments

Background When the connectNow parameter to StreamInterface's init() method is True, it will also call connect() during init(). Then connect() starts a thread for receiving data and starts the process of pulling config info from the node.

The issue When using classes that inherit from StreamInterface it should be possible to instantiate an interface but not "connectNow" and then later call connect() to create a connection.

Currently, when connectNow is false:

  • SerialInterface still creates a serial connection in init() but doesn't call StreamInterface.connect().
  • TCPInterface doesn't create a socket connection in init() but subsequently calling connect() on TCPInterface is ineffective because TCPInterface has its own myConnect() method that is independent of StreamInterface.connect().

Commit 2245ac8d970bf827b32b85eadce6d74ad5f03d2f moves the connection parts of SerialInterface's init() method out into a separate connect() method that does its bit and then calls connect() from StreamInterface (and which won't be called during init() if connectNow is false).

Commit 76418b8e574a3a216a572d6e720c5aa060a35512 similarly adds a connect() method that calls TCPInterface's myConnect() first before calling connect() from StreamInterface.

The other commits are incidental cleanups. f3f17a7d50b7b8f368de488f19b09b054d60916b and 3be73b42e219ab4c5235c77e417c7ee018357008 get rid of some variables that are already better declared elsewhere. 79334e83e6dc94c472a990849701fc372ca609cf and 040f332078085fc04710c63d67388fc0c7f07293 tidy up the way that the use of StreamInterface in a non-abstract way is blocked, by changing how that is checked for and issuing a RuntimeError instead of a generic exception (with a corresponding test case change).

These changes shouldn't break anything, except perhaps if anyone is already using a workaround for the issue in separate apps.

Travis-L-R avatar Nov 11 '25 10:11 Travis-L-R

I think this looks good as long as pylint etc. can be made happy. Thanks for the PR!

ianmcorvidae avatar Nov 12 '25 17:11 ianmcorvidae

Thanks,

pylint is complaining here over the use of type() instead of isinstance(), but using isinstance() would defeat the purpose of checking that it's a StreamInterface object and not a class that inherits from it.

So I've added a pylint disable flag in 1214d5010bcfa8986b6e2797ed5e1a8fc7f20b20

Ideally StreamInterface would be made a proper abstract class, but that'd need a more significant refactor.

Travis-L-R avatar Nov 12 '25 21:11 Travis-L-R