Skip to content

Conversation

@Travis-L-R
Copy link

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 2245ac8 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 76418b8 similarly adds a connect() method that calls TCPInterface's myConnect() first before calling connect() from StreamInterface.

The other commits are incidental cleanups. f3f17a7 and 3be73b4 get rid of some variables that are already better declared elsewhere. 79334e8 and 040f332 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.

@ianmcorvidae
Copy link
Contributor

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

@Travis-L-R
Copy link
Author

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 1214d50

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

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