Skip to content

Fix rx_slip_minus test failure and add BPF test.#2

Merged
peterbmarks merged 8 commits intopeterbmarks:mainfrom
tmiw:main
Mar 15, 2026
Merged

Fix rx_slip_minus test failure and add BPF test.#2
peterbmarks merged 8 commits intopeterbmarks:mainfrom
tmiw:main

Conversation

@tmiw
Copy link
Copy Markdown
Collaborator

@tmiw tmiw commented Mar 12, 2026

Fixes off-by-one issue in the RX buffer handling logic that was causing rx_slip_minus to fail. Some additional minor performance enhancements and cleanup as well as a C version of the original Python BPF tests.

CC @drowe67

@tmiw tmiw mentioned this pull request Mar 12, 2026
@tmiw tmiw changed the title Fix rx_slip_minus test failure. Fix rx_slip_minus test failure and add BPF test. Mar 12, 2026
@tmiw
Copy link
Copy Markdown
Collaborator Author

tmiw commented Mar 12, 2026

Ported over the Python BPF tests from radae/dsp.py. Not sure if those are sufficient but they do pass, anyway.

@drowe67
Copy link
Copy Markdown
Collaborator

drowe67 commented Mar 12, 2026

Good work @tmiw, I'll take a look at it shortly.

@peterbmarks - pls don't merge this until I've had a chance to review.

for (int s = 0; s < Ns; s++) {
/* Interpolation factor: pilot at 0, data at 1..Ns, pilot at Ns+1 */
float t = (float)(s + 1) / (float)(Ns + 1);
float t = (float)(s) / (float)(Ns + 1);
Copy link
Copy Markdown
Collaborator

@drowe67 drowe67 Mar 12, 2026

Choose a reason for hiding this comment

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

This is a very good catch @tmiw. This is just the sort of really subtle bug that scares me with AI based coding. A small degradation in quality that we would then be building upon in future releases.

How did you spot the issue?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Honestly, I was just looking at the generated C code side by side with the original Python to see if there was anything incorrect in the former.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well done!

I was thinking of adding an earlier test where I configure the system to be a QPSK Modem rather than send RADE symbols. This way we can measure BER and let "physics" determine if the EQ is working right. It's really hard to work out if the very complex EQ code (all those Matrix manipulations) is OK by inspection.

This might require some C code support, perhaps Claude can help.

@drowe67
Copy link
Copy Markdown
Collaborator

drowe67 commented Mar 12, 2026

@tmiw - so how do we run the BPF Test? Do we still need to run from something like drowe67/radae#66 ?

{
rade_bpf_process(&bpf, &output[i * samplesPerIteration], &input[i * samplesPerIteration], samplesPerIteration);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Using a loop to process chunks at a time is good, as bugs often occur at the boundaries. Good idea to listen to the output files for any clicks (render as a wav file), and plot, looking for spikes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@tmiw - can you pls add an extra command line argument that saves the output[] to a file, just like the Python version?

I'll then run it locally and do some manual checks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added an additional command line argument at the end, i.e.

/rade_bpf_test 1 8000 output.f32

This saves output prior to the post-processing/FFT step. Let me know if you'd rather have the output from FFT instead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @tmiw - I ran the tests manually, did a few plots from the .c64 files and they look good, no jaggies, e.g. the imag part:

Screenshot from 2026-03-13 12-04-49

This sounds good (no clicks):

cd ~/radae
cat ~/radae_nopy/build/t.c64 | python3 f32toint16.py --scale 8192 | aplay -f S16_LE -c2

@tmiw
Copy link
Copy Markdown
Collaborator Author

tmiw commented Mar 12, 2026

@tmiw - so how do we run the BPF Test? Do we still need to run from something like drowe67/radae#66 ?

Yep (radae_nopy_bpf_*), or you can run rade_bpf_test directly, i.e. ./rade_bpf_test 1 8000. It'll print "PASS" or "FAIL" and additionally return 0 or -1 respectively.

@drowe67
Copy link
Copy Markdown
Collaborator

drowe67 commented Mar 13, 2026

Thanks @tmiw @peterbmarks I am happy with merging this PR.

@tmiw tmiw mentioned this pull request Mar 15, 2026
@peterbmarks peterbmarks merged commit d5cefb1 into peterbmarks:main Mar 15, 2026
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.

3 participants