Skip to content

Make Serial and TCP interfaces more closely respect connectNow when false#857

Merged
ianmcorvidae merged 7 commits into
meshtastic:masterfrom
Travis-L-R:iface_work
May 31, 2026
Merged

Make Serial and TCP interfaces more closely respect connectNow when false#857
ianmcorvidae merged 7 commits into
meshtastic:masterfrom
Travis-L-R:iface_work

Conversation

@Travis-L-R

Copy link
Copy Markdown

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
Copy Markdown
Contributor

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

@Travis-L-R

Copy link
Copy Markdown
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.

@ianmcorvidae ianmcorvidae merged commit 14f5339 into meshtastic:master May 31, 2026
5 of 9 checks passed
@ianmcorvidae

Copy link
Copy Markdown
Contributor

My bad on losing track of this, merged now. Thanks for the PR!

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