-
Notifications
You must be signed in to change notification settings - Fork 87
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
Wait for clear channel (not busy) when using VARA #387
base: develop
Are you sure you want to change the base?
Conversation
Note: I currently only have remote access to radios to test this, which makes generating the right RF rather difficult. But, it seems straight forward enough given the code. |
Updated |
@@ -152,7 +152,7 @@ func Connect(connectStr string) (success bool) { | |||
|
|||
// Wait for a clear channel | |||
switch url.Scheme { | |||
case MethodArdop: | |||
case MethodArdop, MethodVaraFM, MethodVaraHF: | |||
waitBusy(adTNC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must also be changed. Passing adTNC here is only correct for ARDOP. We will need to pass a reference to the appropriate TNC connection, which depend on the URL scheme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I got it. Sorry, i thought "adTNC" was the current one, I see now where the symbols come from.
Sorry, I'm still struggling with idiomatic Go, especially the typing system. (This is the only go code base I've touched).
And yes, to your earlier question, the VARA modem code does implement the Busy() method.
Will send try 3.
Currently, Pat only implements the busy check for ARDOP. Do the same for VARA. The actual busy/not busy logic is already implemented in Pat-Vara/vara.go in handleCmd().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks promising 👍
The only thing remaining is to test this over some time, and under different conditions. Since we don't provide a method for overriding the "wait busy" state, we depend heavily on a good busy detector.
Do you have any chance of testing this soon, or organize some testing one way or another?
Thanks!
Thanks Martin!
I'm away from my radios, but hopefully will be back to them in a few
more weeks.
Benjamin
…On 2/14/23 01:34, Martin Hebnes Pedersen wrote:
***@***.**** commented on this pull request.
Looks promising 👍
The only thing remaining is to test this over some time, and under
different conditions. Since we don't provide a method for overriding
the "wait busy" state, we depend heavily on a good busy detector.
Do you have any chance of testing this soon, or organize some testing
one way or another?
Thanks!
—
Reply to this email directly, view it on GitHub
<#387 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADDPQUAIZS3JHA4RU24NX2TWXM7RTANCNFSM6AAAAAATO7AUEE>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@bseidenberg - Did you get a change to test these changes as we discussed? 🙂 |
Thanks for the reminder! I decided to look at this today, and it
segfaults on a nil pointer exception in transport.go... but I can't
figure out how it's different from the ARDOP version. I need to brush up
on my go pointer passing as I debug...stay tuned.
…On 4/16/23 04:55, Martin Hebnes Pedersen wrote:
@bseidenberg <https://github.com/bseidenberg> - Did you get a change
to test these changes as we discussed? 🙂
—
Reply to this email directly, view it on GitHub
<#387 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADDPQUH3UFJ7FPYEQUQFJZ3XBPMZ5ANCNFSM6AAAAAATO7AUEE>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
OK, I think I understand what's going on...I think. All the other initModem/initTNC() functions are argument-less and directly manipulate the various variables in connect.go for their modems. However initVaraModem() takes a pointer to the modem. That pointer is passed by value as vModem. The vModem local is updated inside initVaraModem() to point to the new modem, but because it's passed by value, the outer "global" of varaHFModem/varaFMModem isn't updated. So, those are always nil, and my code blows up when it tries to dereference them later. This seems like a bug with the call structure of connect.go, and it being inconsistent... thoughts? |
I had a quick look, and I think I've spotted the bug 🙂 The problem is not in your code. The problem is So the global vars I can create a branch to fix this, but I don't have the setup to test. Maybe you could help me with that? What we need to confirm is that the fix does not break teardown on subsequent VARA sessions. E.g. make one connection, wait a bit (without stopping Pat) and then do another connection. |
Yep, happy to test, just shoot me the branch. By the way, if you're on any realtime chat service (IRC?), that'd be great for real-time discussion. Feel free to ping me out of band. (Edit: Also: Glad we agree on the bug :) ) |
Currently, Pat only implements the busy check for ARDOP. Do the same for VARA. The actual busy/not busy logic is already implemented in Pat-Vara/vara.go in handleCmd().