la5nta / wl2k-go

A Winlink framework for Go.
https://getpat.io
MIT License
50 stars 20 forks source link

ardop: Accept bandwidth (`bw`) dial parameter #74

Closed martinhpedersen closed 2 years ago

martinhpedersen commented 2 years ago

An alternative to #73 which reverts the ARQ bandwidth setting on dial error or after the connection is closed.

Felt like I had to pitch in on this one @xylo04, since my request complicated the task quite a bit.

Let me know what you think 🙂

martinhpedersen commented 2 years ago

Sounds good 👍

I'd like to confirm that this is working properly before releasing this. I have only tested the dial timeout case, but not the an actual connection. I don't have access to a HF rig right now. Do you? If not, I guess we could merge and have some of the VARA testers report back.

xylo04 commented 2 years ago

I don't have access to a HF rig right now. Do you?

I'll plan to test this in the next day or two.

xylo04 commented 2 years ago

I'll plan to test this in the next day or two.

I've been so focused on VARA for the past few months that I've lost practice making ARDOP connections. I'm having problems; maybe I'm missing something obvious. I've gone back to Pat's develop branch and even master to make sure recent changes aren't causing issues. Using ardopc dated 2019-09-30 downloaded from @g8bpq's website and rigctld on a Linux x86_64 machine, I'm able to start dialing an ARDOP station and hear the tones on the monitor, but then the radio never comes out of transmit. The tones stop and the radio continues transmitting an unmodulated carrier. I also tried ardopc_64 dated 2018-03-31 and had the same problem.

The same procedure is fine using piardopc on a Raspberry Pi armhf7, it transmits and receives appropriately. I don't think there have been any notable changes to either Pat's ARDOP interface nor ardopc in the past year, so I'm pretty mystified.

xylo04 commented 2 years ago

Using the RPi, I don't see any issue with this PR against the https://github.com/la5nta/pat/tree/develop branch. Next I'm going to see if this PR works with https://github.com/la5nta/pat/pull/332.

xylo04 commented 2 years ago

With a couple of small adjustments to https://github.com/la5nta/pat/pull/332, this PR is setting the bandwidth as intended.

2022/03/29 16:40:02 --> ARQBW
2022/03/29 16:40:02 frame ARQBW 2000MAX
2022/03/29 16:40:02 <-- ARQBW 2000MAX   [ardop.ctrlMsg{cmd:"ARQBW", value:"2000MAX"}]
2022/03/29 16:40:02 --> ARQBW 500MAX
2022/03/29 16:40:02 frame ARQBW now 500MAX
2022/03/29 16:40:02 <-- ARQBW now 500MAX        [ardop.ctrlMsg{cmd:"ARQBW", value:"500MAX"}]

However, at least once I had a connection time out, and the ARQ bandwidth did not reset to its initial value. What's more, since it got stuck in 500 Hz, that became its new default; after successfully making a 2000 Hz connection, it did revert to 500.

2022/03/29 16:45:48 frame BUFFER 0                                                                                           [185/1976]
2022/03/29 16:45:48 <-- BUFFER 0        [ardop.ctrlMsg{cmd:"BUFFER", value:0}]                                                         
2022/03/29 16:45:48 frame NEWSTATE DISC                                                                                                
2022/03/29 16:45:48 <-- NEWSTATE DISC   [ardop.ctrlMsg{cmd:"NEWSTATE", value:0x2}]                                                     
2022/03/29 16:45:48 frame PTT TRUE                                                                                                     
2022/03/29 16:45:48 <-- PTT TRUE        [ardop.ctrlMsg{cmd:"PTT", value:true}]                                                         
2022/03/29 16:45:48 frame PTT FALSE                                                                                                    
2022/03/29 16:45:48 <-- PTT FALSE       [ardop.ctrlMsg{cmd:"PTT", value:false}]                                                        
2022/03/29 16:45:48 frame ARQBW now 500MAX                                                                                             
2022/03/29 16:45:48 <-- ARQBW now 500MAX        [ardop.ctrlMsg{cmd:"ARQBW", value:"500MAX"}]                                           
2022/03/29 16:45:48 Disconnected.

I'll try to gather more information on how the timeout case escaped the defer method.

xylo04 commented 2 years ago

This is the log around the time of the timeout. I would expect to see the defer method kick in and set the bandwidth back to 2000 around this time, but it doesn't:

2022/03/29 16:42:37 frame PTT FALSE                                                                                                    
2022/03/29 16:42:37 <-- PTT FALSE       [ardop.ctrlMsg{cmd:"PTT", value:false}]                                                        
2022/03/29 16:42:39 frame INPUTPEAKS -4789 4391                                                                                        
2022/03/29 16:42:39 <-- INPUTPEAKS -4789 4391   [ardop.ctrlMsg{cmd:"INPUTPEAKS", value:interface {}(nil)}]                             
2022/03/29 16:42:40 frame PTT TRUE                                                                                                     
2022/03/29 16:42:40 <-- PTT TRUE        [ardop.ctrlMsg{cmd:"PTT", value:true}]                                                         
2022/03/29 16:42:42 frame PTT FALSE                                                                                                    
2022/03/29 16:42:42 <-- PTT FALSE       [ardop.ctrlMsg{cmd:"PTT", value:false}]                                                        
2022/03/29 16:42:42 frame DISCONNECTED                                                                                                 
2022/03/29 16:42:42 <-- DISCONNECTED    [ardop.ctrlMsg{cmd:"DISCONNECTED", value:interface {}(nil)}]                                   
2022/03/29 16:42:42 frame STATUS ARQ Timeout from Protocol State:  IDLE                                                                
2022/03/29 16:42:42 <-- STATUS ARQ Timeout from Protocol State:  IDLE   [ardop.ctrlMsg{cmd:"STATUS", value:"ARQ Timeout from Protocol S
tate:  IDLE"}]                                                                                                                         
2022/03/29 16:42:42 Exchange failed: connection lost                                                                                   
2022/03/29 16:42:42 frame BUFFER 0                                                                                                     
2022/03/29 16:42:42 <-- BUFFER 0        [ardop.ctrlMsg{cmd:"BUFFER", value:0}]                                                         
2022/03/29 16:42:42 frame PTT TRUE                                                                                                     
2022/03/29 16:42:42 <-- PTT TRUE        [ardop.ctrlMsg{cmd:"PTT", value:true}]                                                         
2022/03/29 16:42:43 frame PTT FALSE                                                                                                    
2022/03/29 16:42:43 <-- PTT FALSE       [ardop.ctrlMsg{cmd:"PTT", value:false}]                                                        
2022/03/29 16:42:43 frame BUFFER 0                                                                                                     
2022/03/29 16:42:43 <-- BUFFER 0        [ardop.ctrlMsg{cmd:"BUFFER", value:0}]                                                         
2022/03/29 16:42:43 frame NEWSTATE DISC                                                                                                
2022/03/29 16:42:43 <-- NEWSTATE DISC   [ardop.ctrlMsg{cmd:"NEWSTATE", value:0x2}]                                                     
2022/03/29 16:42:43 QSX ardop: 7097.000                                                                                                
2022/03/29 16:42:51 frame {IDF [73 68 58 75 68 48 83 70 89 32 91 68 77 55 56 111 118 93 58]}                                           
2022/03/29 16:42:51 frame INPUTPEAKS -4154 4558                                                                                        
2022/03/29 16:42:51 <-- INPUTPEAKS -4154 4558   [ardop.ctrlMsg{cmd:"INPUTPEAKS", value:interface {}(nil)}]                             
2022/03/29 16:43:01 frame INPUTPEAKS -4474 4111                                                                                        
2022/03/29 16:43:01 <-- INPUTPEAKS -4474 4111   [ardop.ctrlMsg{cmd:"INPUTPEAKS", value:interface {}(nil)}]
xylo04 commented 2 years ago

Near as I can tell, when the ARDOP modem times out, we end up in this switch statement. This branch doesn't trigger tnc.Close(), so the bandwidth reset is never invoked.

martinhpedersen commented 2 years ago

Thanks for testing and debugging 🙇

I've pushed a new commit to make sure we always call conn.Close() after the exchange. Do you mind doing a new test for me when you have the time?

Thanks again! 🙂

PS: We could call the onClose function calls to the main loop of the TNC handler, and trigger it when the TNC state changes from some connected/dialing state to IDLE. But since the main loop of the TNC is complex enough as it is, it would be great to get this approach working IMO.

xylo04 commented 2 years ago

I'm able to start dialing an ARDOP station and hear the tones on the monitor, but then the radio never comes out of transmit.

FYI, many people are experiencing the same problem with ARDOP right now. Root cause unknown.

martinhpedersen commented 2 years ago

FYI, many people are experiencing the same problem with ARDOP right now. Root cause unknown.

Thanks for the heads up. Looks like @km4ack have narrowed it down to an ardopc compatibility issue with the latest kernel. 😞 I guess ardopc locks up while trying to output it's audio, and never sends the PTT FALSE command. Let's hope someone are able to solve it soon, either in the kernel or ardopc 🤞


Did you get a chance to test this any further after my recent changes?

Thanks 🙂

km4ack commented 2 years ago

Sorry. I probably should have cross posted this to the Pat forum as well. I did post to the BPQ forum where John Wiseman hangs out. If I am not mistaken, he is the one that did the initial port of ARDOP for the pi. However, there has been no response posted. My work around can be found in this thread on my forum. Feel free to cross post to the Pat forum. While I have identified the issue, it is above my pay grade to attempt to fix :-)

xylo04 commented 2 years ago

@martinhpedersen I was able to test again tonight, and it looks like the bandwidth reliably resets even after dirty disconnects. This is ready to merge! :rocket:

martinhpedersen commented 2 years ago

@xylo04 Perfect! Thank you for helping me test this 😄