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

Fix interface values not being set on RNodeSubInterface instances #556

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

jacobeva
Copy link
Contributor

@jacobeva jacobeva commented Sep 20, 2024

This one's on me. So, when I designed RNodeMultiInterface, it didn't occur to me that the interface values being set in Reticulum.py would not actually be set until after the class was initialised. And since these values are passed on to the RNodeSubInterface before they're actually initialised, this created a problem. The actual values will never be set in the subinterface (which is the one actually processing packets).

The way this manifested as a bug was in the fact that self.OUT would be set to false on all RNodeSubInterface instances until the interface was destroyed and recreated (i.e. through pulling the connection to the RNode). So, every first connection when the RNS daemon started would result in it not being able to transmit on any of the RNodeSubInterface instances. This would likely have other implications if not fixed as well, such as affecting IFAC or other things which are set on the interface after it is started.

I don't know how you'll feel about the .start() call I made in this PR Mark. It would be entirely possible to create a thread to run in a second or so in the future after the __init__() is called for RNodeMultiInterface in the actual class. That could be done, but I do have reservations about it potentially creating race conditions. At least in this current implementation, it should guarantee this issue does not resurface, but may not be to your liking as of current.

@jacobeva
Copy link
Contributor Author

Sorry for the mess, I accidentally deleted some lines so I had to add them back.

@markqvist markqvist merged commit e6c1dc0 into markqvist:master Oct 5, 2024
1 check passed
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