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

Suppress unnecessary rig_init in hamlib. #173

Merged
merged 1 commit into from
Sep 19, 2021
Merged

Suppress unnecessary rig_init in hamlib. #173

merged 1 commit into from
Sep 19, 2021

Conversation

tmiw
Copy link
Collaborator

@tmiw tmiw commented Sep 15, 2021

Per #172, this only calls rig_init() if the user changes the radio type or on first start.

@drowe67, @Tyrbiter, let me know if this looks okay.

@drowe67
Copy link
Owner

drowe67 commented Sep 15, 2021

Looks good to me @tmiw 👍 It would be good to test this change with a wider audience as part of the next beta, PTT has been a tricky area in the past

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 15, 2021

I'll generate a new test build tonight containing this and #169. Then if good, we can merge all three pending PRs and generate a final 1.6.1 build.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 16, 2021

Just to confirm, this is with Hamlib/Hamlib#797 applied too, right? If I understood the discussion thread correctly, this PR + that patch should have done the trick (although the VFO will still switch to VFO A on first start, just suppressing it on future starts while the app is open).

Anyway, I'd rather not have FreeDV always keep Hamlib connected as that's a more significant design change.

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 16, 2021

BTW, I just emailed out the RC build (based on #168 which contains all current open PRs).

@Tyrbiter
Copy link

Tyrbiter commented Sep 16, 2021

No, I was using the released hamlib-4.3.1, so I will correct that mistake and report back.

And it turns out that the released hamlib-4.3.1 does have Hamlib/Hamlib#797 included in it, which I wasn't expecting.

I think that this can only work if hamlib isn't reopened, what do you think?

A bit more debugging with verbose on reveals that hamlib.cpp:114 has this:

if(m_rig) {
if (g_verbose) fprintf(stderr, "Closing old hamlib instance!\n");
close();
}

So this closes hamlib whatever happens I think.

I have been trying several things to allow the hamlib-4.3.1 change to work, but whatever happens freedv seems to lose the hamlib instance and that means the Hamlib/Hamlib#797 change is removed from its context.

@Tyrbiter
Copy link

I'm with @drowe67 regarding the trickiness of this, so I don't want to hold up the new release and risk breaking this area of the program.

Thinking cap back on...

@tmiw
Copy link
Collaborator Author

tmiw commented Sep 19, 2021

I haven't heard of any issues with the test build, so we can probably merge this now.

+ @drowe67

@drowe67 drowe67 merged commit 7a6ed30 into master Sep 19, 2021
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.

None yet

3 participants