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

Tdh8 duplex adds off option #789

Merged
merged 1 commit into from
Oct 21, 2023
Merged

Tdh8 duplex adds off option #789

merged 1 commit into from
Oct 21, 2023

Conversation

Sandmann34
Copy link
Contributor

Tdh8 duplex adds off option

@@ -1018,6 +1021,8 @@ def set_memory(self, mem):
_mem.txfreq = (mem.freq + mem.offset) / 10
elif mem.duplex == "-":
_mem.txfreq = (mem.freq - mem.offset) / 10
elif mem.duplex == 'off':
_mem.txfreq = 166666665
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is larger than can be stored in the field, so it gets truncated. It also looks like an overflow value. What does the radio really want here? 0? 0xFFFF?

@kk7ds
Copy link
Owner

kk7ds commented Oct 19, 2023

This should say "Related bug #10756" in the commit message to tag this to that open issue. That issue proposes 0xFF for each byte, and that would be much cleaner than what you have here. If that's suitable, please change to use that. You might also implement the mic gain as requested in that issue (and may be fixed in the same file too).

As you see from the failed check, you need to rebase your set on master, not include the merge commits.

Thanks!

Related to bug #10756
@kk7ds kk7ds merged commit 2c99778 into kk7ds:master Oct 21, 2023
6 checks passed
@kk7ds
Copy link
Owner

kk7ds commented Oct 21, 2023

I rebased this for you and tagged the bug. If the right thing to do is use 0xFF for the sentinels, please submit another PR (or confirm and I'll do it).

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