-
Notifications
You must be signed in to change notification settings - Fork 560
IF tap sync support and example #1422
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
Open
Dinsmoor
wants to merge
2
commits into
gqrx-sdr:master
Choose a base branch
from
Dinsmoor:if_tap_sync
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,216 @@ | ||
#!/usr/bin/env python3 | ||
|
||
import sys | ||
import argparse | ||
import socket | ||
import time | ||
|
||
# Constants for IF offsets based on mode | ||
USB_IF_OFFSET = -1500 # USB shifts IF down by 1.5kHz | ||
LSB_IF_OFFSET = 1500 # LSB shifts IF up by 1.5kHz | ||
AM_FM_IF_OFFSET = 0 # No shift for AM/FM | ||
HELP_TEXT = ''' | ||
This panadapter script is designed for keeping Gqrx's waterfall | ||
synchronized with a FT-991/A that has had its IF tapped. | ||
|
||
You must have CAT control of your radio to get the dial frequency. | ||
In this case, the command may be `rigctld -m 1035 -r /dev/ttyUSB0` | ||
|
||
Some notes about the FT-991/A: | ||
Because the IF of the FT-991/A is inverted, in the Input Controls | ||
section of Gqrx, you need to check 'Swap I/Q' | ||
The IF is adjusted by the radio by 1.5khz in each sideband mode, but | ||
this script takes care of that for you. | ||
|
||
Feel free to modify this script for your radio's IF and related quirks! | ||
''' | ||
|
||
def get_rig_freq(rs): | ||
"""Get frequency and mode from the rig""" | ||
rs.send(b'm\n') | ||
mode_response = rs.recv(1024).decode().strip() | ||
mode = mode_response.splitlines()[0] if mode_response else "USB" | ||
is_usb = "USB" in mode.upper() | ||
is_lsb = "LSB" in mode.upper() | ||
is_am_fm = mode in ["AM", "FM", "WFM"] | ||
|
||
rs.send(b'f\n') | ||
freq_response = rs.recv(1024).decode().strip() | ||
freq = int(freq_response) if freq_response.isdigit() else 0 | ||
|
||
return (freq, mode, is_usb, is_lsb, is_am_fm) | ||
|
||
def get_gqrx_hw_freq(gs): | ||
"""Get the current hardware tuning frequency from GQRX""" | ||
gs.send(b'gqrx_get_hw_freq\n') | ||
response = gs.recv(1024).decode().strip() | ||
print(response) | ||
try: | ||
return int(response) | ||
except: | ||
return 0 | ||
|
||
def set_gqrx_hw_freq(gs, freq): | ||
"""Set the hardware tuning frequency in GQRX""" | ||
set_gqrx_lnb_lo(gs, 0) # initialize to zero first, otherwise side effects of | ||
< 10000 td class="blob-code blob-code-addition js-file-line"> # updating the UI will cause setting the frequency to be incorrect. | ||
# we can just set it back afterwards if we want. | ||
set_gqrx_filter_offset(gs, 0) | ||
msg = f'gqrx_set_hw_freq {freq}\n'.encode() | ||
print(msg) | ||
gs.send(msg) | ||
res = gs.recv(1024).decode().strip() | ||
return res | ||
|
||
def get_gqrx_filter_offset(gs): | ||
"""Get the filter offset from GQRX""" | ||
gs.send(b'gqrx_get_filter_offset\n') | ||
response = gs.recv(1024).decode().strip() | ||
parts = response.split() | ||
if len(parts) > 1 and parts[-1].lstrip('-').isdigit(): | ||
return int(parts[-1]) | ||
return 0 | ||
|
||
def set_gqrx_filter_offset(gs, freq_hz): | ||
msg = f'gqrx_set_filter_offset {freq_hz}\n' | ||
gs.send(msg.encode()) | ||
response = gs.recv(1024).decode().strip() | ||
return | ||
|
||
def get_gqrx_freq(gs): | ||
"""Get the demodulation frequency from GQRX""" | ||
gs.send(b'f\n') | ||
response = gs.recv(1024).decode().strip() | ||
return int(response) if response.isdigit() else 0 | ||
|
||
def set_gqrx_freq(gs, freq): | ||
"""Set the demodulation frequency in GQRX""" | ||
gs.send(f'F {freq}\n'.encode()) | ||
return gs.recv(1024).decode().strip() | ||
|
||
def calc_lnb_lo(dial_freq, if_freq, mode): | ||
"""Calculate the LNB_LO value based on dial frequency, IF, and mode""" | ||
# Determine the mode-specific IF offset | ||
if mode == "USB": | ||
mode_offset = USB_IF_OFFSET | ||
elif mode == "LSB": | ||
mode_offset = LSB_IF_OFFSET | ||
else: # AM, FM, etc. | ||
mode_offset = AM_FM_IF_OFFSET | ||
|
||
# For FT991A IF at 69.45MHz: | ||
# If dial is at 144.000MHz, we want a signal at 144.000MHz to appear at 69.45MHz | ||
# LNB_LO should be: 144000000 - 69450000 = 74550000 | ||
# This is essentially: dial_freq - if_freq - mode_offset | ||
lnb_lo = dial_freq - if_freq - mode_offset | ||
|
||
return lnb_lo | ||
|
||
def set_gqrx_lnb_lo(gs, lnb_lo): | ||
"""Set the LNB_LO value in GQRX""" | ||
gs.send(f'LNB_LO {lnb_lo}\n'.encode()) | ||
return gs.recv(1024).decode().strip() | ||
|
||
def freq_to_string(freq): | ||
"""Format frequency for display""" | ||
freq_str = str(freq).rjust(9, '0') | ||
return freq_str[0:3] + "." + freq_str[3:6] + "." + freq_str[6:9] | ||
|
||
def main(): | ||
parser = argparse.ArgumentParser() | ||
parser.add_argument('-ga', '--gqrx-address', type=str, default='localhost', | ||
help='address that Gqrx is listening on') | ||
parser.add_argument('-gp', '--gqrx-port', type=int, default=7356, | ||
help='remote control port configured in Gqrx') | ||
parser.add_argument('-ra', '--rigctld-address', type=str, default='localhost', | ||
help='address that rigctld is listening on') | ||
parser.add_argument('-rp', '--rigctld-port', type=int, default=4532, | ||
help='listening port of rigctld') | ||
parser.add_argument('-i', '--interval', type=float, default=1.0, | ||
help='update interval in seconds') | ||
parser.add_argument('-f', '--ifreq', type=float, default=69.45, | ||
help='intermediate frequency in MHz') | ||
parser.add_argument('--debug', action='store_true', | ||
help='Enable debug output') | ||
|
||
args = parser.parse_args() | ||
debug = args.debug | ||
if_freq = int(args.ifreq * 1e6) # Convert to Hz | ||
|
||
print(HELP_TEXT) | ||
|
||
try: | ||
rs = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
rs.connect((args.rigctld_address, args.rigctld_port)) | ||
except Exception as e: | ||
print(f'Connection to rigctld failed: {e}', file=sys.stderr) | ||
return 1 | ||
|
||
try: | ||
gs = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
gs.connect((args.gqrx_address, args.gqrx_port)) | ||
except Exception as e: | ||
print(f'Connection to Gqrx failed: {e}', file=sys.stderr) | ||
return 1 | ||
|
||
try: | ||
# Always set the hardware frequency to IF frequency | ||
if debug: | ||
print(f"Setting hardware frequency to IF: {if_freq} Hz") | ||
|
||
set_gqrx_hw_freq(gs, if_freq) | ||
|
||
old_mode = "" | ||
old_rig_freq = -1 | ||
|
||
while True: | ||
# Get the current rig frequency and mode | ||
(rig_freq, mode, is_usb, is_lsb, is_am_fm) = get_rig_freq(rs) | ||
|
||
# Get the current hardware frequency from GQRX | ||
hw_freq = get_gqrx_hw_freq(gs) | ||
if not hw_freq: | ||
time.sleep(args.interval) | ||
continue | ||
filter_offset = get_gqrx_filter_offset(gs) | ||
|
||
if debug: | ||
print(f"\nRig Frequency: {freq_to_string(rig_freq)} Hz, Mode: {mode}") | ||
print(f"GQRX Hardware Frequency: {freq_to_string(hw_freq)} Hz") | ||
print(f"GQRX Filter Offset: {filter_offset} Hz") | ||
|
||
# If the rig frequency or mode has changed, update the LNB_LO | ||
if rig_freq != old_rig_freq or mode != old_mode: | ||
# Calculate the correct LNB_LO based on dial frequency and mode | ||
lnb_lo = calc_lnb_lo(rig_freq, if_freq, mode) | ||
|
||
# Make sure the hardware frequency is still at the IF | ||
if hw_freq != if_freq: | ||
if debug: | ||
print(f"Resetting hardware frequency to IF: {if_freq} Hz") | ||
set_gqrx_hw_freq(gs, if_freq) | ||
|
||
if debug: | ||
print(f"Updating LNB_LO to: {lnb_lo} Hz") | ||
# Set the LNB_LO in GQRX | ||
set_gqrx_lnb_lo(gs, lnb_lo) | ||
|
||
# Update last values | ||
old_rig_freq = rig_freq | ||
old_mode = mode | ||
|
||
# Wait for the next update interval | ||
time.sleep(args.interval) | ||
|
||
except KeyboardInterrupt: | ||
if debug: | ||
print("\nProgram terminated by user") | ||
except Exception as e: | ||
print(f'Unexpected error: {e}', file=sys.stderr) | ||
return 1 | ||
finally: | ||
rs.close() | ||
gs.close() | ||
|
||
if __name__ == '__main__': | ||
sys.exit(main() or 0) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 sounds ugly. Is there a way to make this work better?
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.
It is ugly. Just like me. 👴 - Thanks for looking at it regardless.
Probably! In the context of this repository: I have no idea what I'm doing. That's why I asked for a contribution guide! 🙂
Once I have some fresh eyes, I will look at it again. I was just happy that it worked for me, finally.
Do you have an opinion on how that should be implemented instead?
I am not used to the github culture - I suppose a 'draft PR' would have been better - as I am not 100% certain how each element of the features I want to make should be implemented, mostly because of a 'forest for the trees' sort of situation where I only have so much time in the day to study and understand a repository. Looking back it's obvious: a PR means "This code is ready and can be merged now, in the author's opinion."
Some thoughts I have thought about over the last few days:
Maybe there is a more fundamental architectural problem in Gqrx? Why does updating the widgets cause a signal to be sent to setNewFrequency again (if the functions that send the signals are called from there to begin with?) - if they really are supposed to update the UI? Aren't Qt signals and modules meant to reduce the interdependence of sections of code and make it more managable?
How much of a role should be placed on an external script communicating via a socket vs just implementing the logic in the program as a feature?
Why am I so dumb?
But it's more likely that I, again, don't understand how it's written. I was happy I was able to get it to work, but would also like to learn how to work together in a repository.
I'll look at it again with fresh eyes later. At the very minimum, at least knowing the current hardware frequency is absolutely required for making a feature like this work - but I'd rather it feel like less of a "hack" and more of a "well designed feature" - I'll get back to you sometime soon.
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.
Draft makes sense if you're still planning to do some further work or cleanup, and want to get some early feedback on your work-in-progress. Anyway, it doesn't matter too much for a small project like Gqrx where it will be clear from the discussion thread whether the code is ready for a final review.
There are definitely architectural problems in Gqrx. It's a hobby project that has been developed over time by volunteers whose specialty is signal processing and not software engineering.
To be honest, I don't know Qt that well. The changes I have made since taking over as maintainer have mostly been small enough that I've been able to copy over existing patterns.
I do know that signals can be temporarily blocked, if necessary, with
blockSignals
, and Gqrx does this in various places.Good question. Both are possible, and each option has different advantages and disadvantages.
Building in a new feature requires ongoing maintenance and testing. It makes sense to do that if the feature will be used by a large enough subset of users, and there is someone willing to keep the feature maintained.
I don't. But I do expect that one way or another the limitations on
gqrx_set_hw_freq
could be removed, thereby making the command easier to use.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.
I spent some time looking at it again. Without significantly restructuring how Gqrx handles its UI update methods, as a very major refactor that would require strongly separating the UI from IO (a good idea - just well beyond this PR's scope and somthing that I don't think any of us have time for), I think my commits are the most reasonable compromise to get the desired IF tap panadapter functionality to Gqrx, with the least amount of changes to Gqrx, meaning the least amount of 'messing' with how Gqrx works internally, and provides a working example with reasonable inline documentation for those who would desire this feature for their own radios. It may not be very many people who have my use case, but I will share with sdr-kits that it's a possibility with Gqrx, if merged.
Additionally, I was looking at the open PR history - and there are a lot of really cool PRs! But most significantly change how Gqrx works internally, and are unlikely to be merged because of both that by itself, relatively poor internal documentation, and relatively poor (at least publically facing) communication with maintainers.
Naturally I'd rather just provide this feature WITHIN Gqrx with no need for an external interpreted script - but the archetecture of Gqrx is too delicate to permit it without major refactor, in my brief experience with the program and my own experimentation over the last year or so. Without authoritative guidance for future development, the reasonable default is to make as little of changes as possible, in the same spirit and style as what already exists.
At this time, I think it's reasonable to have most of the work done in the script, although I certainly agree that
gqrx_set_hw_freq
should just do what it says on its face and feel less hacky... - it also currently has documentation to say what needs to be done first to be deterministic.TL;DR: I don't feel comfortable messing too much with how Gqrx works internally to support this feature when an external script and some inline documentation work fine to do the job.
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.
What do you think about enabling the "discussions" feature for this repository, so something like a documentation of the existing gqrx repository can be analyzed and broken down into functional descriptive sections?