la5nta / wl2k-go

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

Extend rigctld.go for la5nta/pat issue #185 #61

Closed DougCollinge closed 3 years ago

DougCollinge commented 3 years ago

Extend rigctld.go to handle hamlib calls with multiple outputs, like get_mode. The unpatched version assumes that all hamlib calls return only one output.

DougCollinge commented 3 years ago

I meant to base this on the 'develop' branch but I don't know how to change it. It probably makes no difference, since there are no conflicts.

martinhpedersen commented 3 years ago

@DougCollinge - First of all, thank you for working on this. It is highly appreciated!

I have some concerns regarding these changes though. The main thing is that the hamlib package provides both rigctld and direct C bindings (libhamlib) over the same set of interfaces. This PR only address the rigctld part, which breaks compilation of libhamlib.go.

Are you interested in porting these changes to libhamlib.go as well? If so, I'll provide a proper review of what you've done so far and we can go from there.

Thanks!

DougCollinge commented 3 years ago

I am using this patch in my personal build and it works pretty well but I think we should maybe discuss the strategy a little before I develop it much more. Yes, I am willing to support the hamlib bindings as well as rigctld, I just felt like I was getting pretty far out on a limb so I stopped until you got involved.

The approach I took was to avoid stepping on any toes by not changing existing code too much, just adding new stuff. The result is clumsy IMHO. I think maybe a better way to do it is to use your "VFOmode" and translate the results to string using an added map. I could then take out the GetModeAsString() and presumably that would fix the problem with libhamlib.

There is also the possibility of collecting the capabilities of the rig when it is initialized, so that inappropriate mode settings can be caught immediately, or prevented with a dropdown control in the UI. Having the rig capabilities map around might be useful for other features as well.

DougCollinge commented 3 years ago

Oh yeah, I forgot: there's a bugfix buried in there, too. Check out lines 61-63 in rigctl.go.

If you wish, I could split this fix out into a separate commit so you could cherry-pick it, or else it could go into a whole new PR. Whatever.

mattwood2000 commented 2 years ago

+1 for this. I have been putting together an emcom setup and was just about to tackle the problem of switching between ARDOP on HF and AX25 on VHF. Mode switch is really a necessary component. Will apply your patch for now until the strategy and functionality is mainlined.