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

JT4 enablement #374

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

JT4 enablement #374

wants to merge 9 commits into from

Conversation

sq6emm
Copy link

@sq6emm sq6emm commented Feb 28, 2024

This is fixing #373

Screenshot from 2024-02-28 23-17-18

Copy link
Owner

@jketterl jketterl left a comment

Choose a reason for hiding this comment

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

Thank you for the pull request. The change looks good overall, just needs a final polishing pass, please see inline comments. Please also check the question about a potential impact on CPU usage by the increase in frequency tolerance. Cheers!

owrx/wsjt.py Outdated
def getInterval(self):
return 60

def getSubmode(self):
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see this method being used anywhere. Any plans for this? If not, I'd recommend removing it.

Copy link
Author

@sq6emm sq6emm Mar 3, 2024

Choose a reason for hiding this comment

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

It is needed by audio/chopper.py

Copy link
Owner

Choose a reason for hiding this comment

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

It isn't. Please remove.

Copy link
Owner

Choose a reason for hiding this comment

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

Right... so... I saw the other comment, I believe it went a bit astray. I'm gonna stay on this thread in an attempt to keep things separate.

I also think I see what the issue is here, it's a simple misunderstanding. In pull requests, you usually put comments on the offending line, or the line that best matches the topic to be discussed. In this case, I put the initial comment on the line that has the getSubMode() method signature, trying to tell you that it isn't used.

getInterval() is indeed used, it is part of the AudioChopperProfile API, but I wasn't referring to that one 😉

Copy link
Author

Choose a reason for hiding this comment

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

Corrected;)

@sq6emm
Copy link
Author

sq6emm commented Jun 11, 2024

@jketterl how do you think, is it now old enough to be merged?:)

@jketterl
Copy link
Owner

Has there been any updates?

@sq6emm
Copy link
Author

sq6emm commented Jun 12, 2024

Well I believe for basic functionality nothing more needed? Decoding of JT4 is currently based on the default bandwidth. I also checked as you requested the CPU Increase and described them so you can decide if to implement a possibility to modify the decode bandwidth (that I consider as a feature update)... So well is there anything missing?

@jketterl
Copy link
Owner

Your initial response to the discussion about the -F parameter was:

This is needed capability especially on uWaves where frequencies are not stable enough.

This gave me the impression that being able to control the frequency tolerance was essential to this feature. As such, I did not consider merging this for as long as this setting is not available.

@sq6emm
Copy link
Author

sq6emm commented Jun 15, 2024

It's true, but with agile approach it's better to have a MVP;)

@sq6emm
Copy link
Author

sq6emm commented Oct 16, 2024

@jketterl any chance to merge it?

@jketterl
Copy link
Owner

Do you see any change? Did you do any changes yourself?

@sq6emm sq6emm closed this Oct 16, 2024
@jketterl jketterl reopened this Oct 16, 2024
@sq6emm
Copy link
Author

sq6emm commented Feb 24, 2025

Hey @jketterl I have added a generic possilbilty for wsjt modes to set frequency tolerance - only interpreted by JT4 now.
Please tell me if its okey for you now - if so I will extended that parameter to all wsjt modes and push.

@jketterl
Copy link
Owner

Looks alright on first glance, but I'd prefer it to be labeled "JT4" both visible and internally. It only applies to JT4 modes (at least that's what I understand, and that's also what I'd expect).

@sq6emm
Copy link
Author

sq6emm commented Feb 24, 2025

No, the feature of frequency tolerance is generic to all jt9 related modes.

@jketterl
Copy link
Owner

Ah, now I see, you wanted to extend this to all WSJT-X modes. Any reason to do so? I don't see any problems with the current default, at least not for the existing modes. I feel like this will only cause additional CPU overhead when used, and you'll probably need a separate setting for each mode. Why not limit this to JT4 (where it's needed)?

@sq6emm
Copy link
Author

sq6emm commented Feb 24, 2025

Okey, will limit that to JT4 and Q65 if its okey for you?

@jketterl
Copy link
Owner

once again, why Q65?

@sq6emm
Copy link
Author

sq6emm commented Feb 24, 2025

both Q65 and JT4 are often used by microwave beacons... where freq stability is not so good.

@sq6emm
Copy link
Author

sq6emm commented Feb 24, 2025

Let me be more precise WHY,

common setup to receive 10GHZ with openwebsdr is rtldongle + LNB from SAT TV. LNBs usually dont have stable frequency for SSB and for narrow modes its even more an issue.

Plus on microwaves with digi often beacons are received through rain scatter or air plane scatter - both impacting the receiving frequency. So that frequency tolerance is important on microwaves to compensate those situations.

True, I do not see any usage today beyond JT4 (g) or Q65 (c,d,e) so I would propose I will change this setting to JT4 and Q65 frequency tolerance. And apply that only to those if it will be ok for you.

@sq6emm
Copy link
Author

sq6emm commented Mar 6, 2025

@jketterl any comment, suggestion so I can adjust it according to your comfort?

@jketterl
Copy link
Owner

jketterl commented Mar 7, 2025

Apologies, I thought I had sent a reply already. Probably typed something and lost it in some buried browser tab.

For general practice, I'd say it would be better to have a separate pull request for Q65, mostly because there should be no depencencies here, I'd say both modes should have an independent setting.

Other than that this looks alright to me.

@sq6emm
Copy link
Author

sq6emm commented Mar 10, 2025

Just a kindly reminder;)

owrx/wsjt.py Outdated
def getInterval(self):
return 60

def getSubmode(self):
Copy link
Owner

Choose a reason for hiding this comment

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

It isn't. Please remove.

owrx/wsjt.py Outdated
@@ -29,6 +29,13 @@ def decoding_depth(self):
# default when no setting is provided
return 3

def jt4_frequency_tolerance(self):
Copy link
Owner

Choose a reason for hiding this comment

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

This has no business being on WsjtProfile if it's only used in Jt4Profile. Please move.

Copy link
Author

Choose a reason for hiding this comment

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

getInterval is used - as this mode is decoded every 60 seconds, if removed:

Traceback (most recent call last):
  File "/usr/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
    self.run()
  File "/opt/openwebrx/owrx/audio/chopper.py", line 59, in run
    self.setup_writers()
  File "/opt/openwebrx/owrx/audio/chopper.py", line 48, in setup_writers
    sorted_profiles = sorted(self.profile_source.getProfiles(), key=lambda p: p.getInterval())
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/openwebrx/owrx/wsjt.py", line 79, in getProfiles
    return [JT4Profile(i) for i in profiles if i in JT4Profile.availableSubmodes]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/openwebrx/owrx/wsjt.py", line 79, in <listcomp>
    return [JT4Profile(i) for i in profiles if i in JT4Profile.availableSubmodes]
            ^^^^^^^^^^^^^
TypeError: Can't instantiate abstract class JT4Profile with abstract method getInterval

Copy link
Author

Choose a reason for hiding this comment

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

This has no business being on WsjtProfile if it's only used in Jt4Profile. Please move.

Corrected.

@jketterl
Copy link
Owner

Also, I just ran this with on an rtl-sdr with no real reception. There's nothing on the waterfall. Yet, for some reason, I have empty decodes showing up in the receiver. I'm not sure if this is false positives, or some kind of quirk of the JT4 decoder, but if possible, this should be accounted for.

localhost_8073_ (17)

@sq6emm
Copy link
Author

sq6emm commented Mar 18, 2025

Also, I just ran this with on an rtl-sdr with no real reception. There's nothing on the waterfall. Yet, for some reason, I have empty decodes showing up in the receiver. I'm not sure if this is false positives, or some kind of quirk of the JT4 decoder, but if possible, this should be accounted for.

This is happening due to strange jt9 behaviour. I confirmed it also using recordings from owrx outside of containers on my local computer:

$ jt9 -4 -b G copy-openwebrx-audiochopper-126863547833744-250318_2144.wav
2144 -26  4.3 1488 $
<DecodeFinished>   0   1        0

this is with just background noise
copy-openwebrx-audiochopper-126863547833744-250318_2144.wav.gz

I have asked for it on wsjt group. (https://wsjtx.groups.io/g/main/message/55445)

@jketterl
Copy link
Owner

Thanks for inquiring about the empty messages. Let's see what comes out of it. I'm not subscribed to that mailing list, so feel free to ping here if there's any updates.

Signed-off-by: Dawid SQ6EMM <[email protected]>
@sq6emm
Copy link
Author

sq6emm commented Mar 20, 2025

I suggest we leave it as it is - native behaviour.
Unless you have another idea @jketterl ? (script that will parse the output)

image

image

@sq6emm
Copy link
Author

sq6emm commented Mar 20, 2025

This is the proposed script that will replace jt9 command for jt4 (if you think it make sense)

Characters that need to be taken into account as end of message are: f, fN, dCN - not only "f" like in an example. Following wsjt-x Use Guide - 13.2 Decoded Lines Table 5 (Notations used on decoded text lines).

cat jt9-4wrap.sh 
#!/bin/bash

jt9 ${@} | egrep -e 'f  $' -e '^<'

and output of execuction:

$ ./jt9-4wrap.sh -4 -b G -d 3 -F 1000 openwebrx-audiochopper-126863547460368-250318_2137.wav
<DecodeFinished>   0   1        0
$ ./jt9-4wrap.sh -4 -b G -d 3 -F 1000 openwebrx-audiochopper-126863547460368-250318_2138.wav
2138 -14 -0.6  888 $* SR6LEG JO81CE          f 
<DecodeFinished>   0   1        0

unless you prefer to leave it untouched.

I have an idea how to do the same inside of WsjtParser() - but first decision needed how it should behave;)

@jketterl
Copy link
Owner

Well, I'm afraid I don't think that adding a script for this purpose makes sense. Not only does this add a new dependency (egrep), there's also the problem of how to distribute that script.

If you want to filter those messages I recommend having a look at the existing parser code inside OpenWebRX.

You have a new reply by Joe Taylor now to explain what those messages are.

@sq6emm
Copy link
Author

sq6emm commented Mar 20, 2025

Ok, now it does not bother us with empty messages

image

@sq6emm
Copy link
Author

sq6emm commented Mar 25, 2025

Do we miss anything yet?

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