-
Notifications
You must be signed in to change notification settings - Fork 302
Plumb timeout from --timeout through MeshInterface #839
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
Conversation
3a18923 to
c2cfd9a
Compare
|
Thanks for this. Looks pretty good to me, seeing if CI catches anything but otherwise should be good to go I think. |
c2cfd9a to
346d7c5
Compare
Fix C0301: Line too long Ignore the pylint for 6 positional arguments
346d7c5 to
2de7c30
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #839 +/- ##
=======================================
Coverage 59.99% 59.99%
=======================================
Files 24 24
Lines 4267 4267
=======================================
Hits 2560 2560
Misses 1707 1707
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
When is a --timeout command likely to be rolled out to a firmware version and documentation? |
It's not a new feature of the CLI or related to the firmware. It's the timeout on waiting for a response from a node. It fixes a bug that made MeshInterface use the default 20 second timeout, making it difficult to administer remote nodes over a few hops. |
|
I have a small fleet of Heltec V3s, the cheapest of the bunch, and likely the slowest. I use a bash script to test and exercise them over tcp Between each command set is a sleep of 60 seconds. Nevertheless, for anything beyond a broadcast or direct message, say an acknowledgement or traceroute, it's more likely I get a timeout than a response. My interpretation of the thread above is that the Python standard timeout is 20 seconds and you were suggesting the timeout period to be user-adjustable as a preference. I frankly don't know whether this is a function of the device firmware or the meshtasticd library. |
|
I see, the bug has affected you too. If you run This is as opposed to timing out after 20 seconds and not being controllable by the --timeout parameter, which is the cause of "it's more likely I get a timeout than a response" if you're expecting a successful traceroute. This is fully a bug in the meshtastic CLI not waiting long enough. |
|
So is If it's a part of meshtasticd, it would be nice if It would be nicer still if |
|
I think there's been some confusion about and nature of this PR and the names and purposes of meshtastic tools. This fix is solely for the Meshtastic Python CLI tool. It is not a firmware change, and it’s not related to meshtasticd. Just to clarify:
Regarding making --timeout persistent or user-configurable in the device: To reiterate, this PR:
I’d like to keep the scope here focused on resolving #838. |
Resolves conflicts in: - ble_interface.py: Add timeout parameter to __init__ signature - mesh_interface.py: Update docstring to include timeout parameter Master changes included: - Plumb timeout from --timeout through MeshInterface (2de7c30) - Merge pull request meshtastic#839 (dcd077d)
This should fix #838. It adds a timeout parameter to the tcp, ble, stream, and serial interfaces, which gets passed down to MeshInterface directly or through StreamInterface. I tested a short and long timeout through serial and tcp and it honors it now.