Merge dr-qam16-cport into main#1
Conversation
…kets mpp at 15dB-ish SNR
Resolve src/ofdm_mode.c merge conflict while preserving both qam16 and qam16c2 mode configuration.
There was a problem hiding this comment.
Pull request overview
This PR merges the dr-qam16-cport branch into main, extending the existing OFDM/LDPC stack to support a new high-SNR 16-QAM OFDM data mode (qam16c2) and generalizing several QPSK-specific paths to handle bps=4.
Changes:
- Add
qam16c2mode plumbing across C modem, FreeDV API/raw tools, and Octave reference scripts; increase UW capacity to support larger UW bitfields. - Generalize packet assemble/disassemble, demod, and LDPC soft-decision (LLR) generation from QPSK-only to PSK/QAM (2 or 4 bits/symbol).
- Add/adjust CMake tests and documentation for the new mode.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| unittest/tqam16.c | Updates qam16 demod call signature in unit test. |
| unittest/tofdm.c | Updates Demod2D call signature and modem packet assemble helper. |
| stm32/unittest/src/tst_ofdm_mod.c | Refactors formatting/includes; switches to PSK packet assembler. |
| stm32/unittest/src/tst_ofdm_demod.c | Refactors formatting/includes; switches to PSK packet disassembler and updated LLR flow. |
| src/reliable_text.c | Updates symbols_to_llrs() call to include bps. |
| src/ofdm_mode.c | Adds EsNodB to configs and introduces qam16c2 mode parameters/UW. |
| src/ofdm_mod.c | Switches to PSK packet assembler. |
| src/ofdm_internal.h | Raises MAX_UW_BITS, adds EsNodB fields, updates function prototypes for generalized PSK/QAM paths. |
| src/ofdm_demod.c | Uses EsNodB->linear EsNo conversion; switches to PSK disassembly; updates demod/error-count paths for bps=4. |
| src/ofdm.c | Adds amplitude-normalized qam16 demod, generalized UW symbol creation/mod/demod to bps=2/4, and new PSK packet helpers. |
| src/mpdecode_core.h / src/mpdecode_core.c | Generalizes Demod2D/symbol-to-LLR pipeline to variable constellation sizes (QPSK/QAM16). |
| src/interldpc.h / src/interldpc.c | Generalizes LDPC interleave TX modulation and uncoded error counting for bps=2/4. |
| src/freedv_* | Adds new FreeDV mode constant and routes OFDM data open/tx/rx through qam16c2. |
| octave/* | Updates Octave mode/config scripts and QAM16 test to match C port changes. |
| README_data.md | Documents the new QAM16C2 mode in the data mode table. |
| CMakeLists.txt | Adds qam16c2 test coverage (Octave Tx/C Rx and C Tx/C Rx). |
Comments suppressed due to low confidence (1)
src/ofdm_demod.c:530
- In the non-LDPC (hard decision) path, the code always calls
qpsk_demod()and writes only two bits per symbol (bits[1]/bits[0]). Forofdm->bps == 4(e.g. qam16c2) this will generate incorrect output and leave parts ofrx_bits_charuninitialized. Either gate this branch with anassert(ofdm->bps == 2)or add the corresponding 16-QAM demod + bit packing logic forbps == 4.
for (i = 0; i < Npayloadsymsperpacket; i++) {
int bits[ofdm->bps];
complex float s = payload_syms[i].real + I * payload_syms[i].imag;
qpsk_demod(s, bits);
rx_bits_char[ofdm_config->bps * i] = bits[1];
rx_bits_char[ofdm_config->bps * i + 1] = bits[0];
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| unsigned char tx_bits_char[]); | ||
| void qpsk_modulate_frame(COMP tx_symbols[], int codeword[], int n); | ||
| void ldpc_decode_frame(struct LDPC *ldpc, int *parityCheckCount, int *iter, | ||
| uint8_t out_char[], float llr[]); | ||
| int count_uncoded_errors(struct LDPC *ldpc, struct OFDM_CONFIG *config, |
There was a problem hiding this comment.
interldpc.c renamed qpsk_modulate_frame() to psk_modulate_frame(int bps, ...), but this header still declares qpsk_modulate_frame(...). Please update the header to match the implementation (rename the prototype or provide a compatibility wrapper) to avoid stale/incorrect declarations for users of interldpc.h.
| int i; | ||
|
|
||
| float symbol_likelihood[nsyms * QPSK_CONSTELLATION_SIZE]; | ||
| float bit_likelihood[nsyms * QPSK_BITS_PER_SYMBOL]; | ||
|
|
||
| Demod2D(symbol_likelihood, rx_qpsk_symbols, S_matrix, EsNo, rx_amps, mean_amp, | ||
| nsyms); | ||
| Somap(bit_likelihood, symbol_likelihood, QPSK_CONSTELLATION_SIZE, | ||
| QPSK_BITS_PER_SYMBOL, nsyms); | ||
| for (i = 0; i < nsyms * QPSK_BITS_PER_SYMBOL; i++) { | ||
| int constellation_points = 1 << bps; | ||
| float symbol_likelihood[nsyms * constellation_points]; | ||
| float bit_likelihood[nsyms * bps]; | ||
|
|
There was a problem hiding this comment.
symbol_likelihood and bit_likelihood are VLAs sized by nsyms * (1<<bps) and nsyms * bps. With bps=4 and large frames (e.g. 16200-bit LDPC → nsyms=4050), this is >300KB of stack per call and can overflow on smaller stacks (or in embedded/threaded contexts). Consider allocating these buffers on the heap, reusing a preallocated workspace, or otherwise bounding stack usage.
| config->clip_en = false; | ||
| config->tx_bpf_en = false; |
There was a problem hiding this comment.
In the qam16c2 mode block, clip_en (and tx_bpf_en just below) are assigned multiple times to the same value. Please remove the redundant assignments to keep the mode configuration concise and reduce the chance of future inconsistencies when tuning parameters.
| config->clip_en = false; | |
| config->tx_bpf_en = false; |
| complex float symbol = qam16_mod(tx_bits); | ||
| qam16_demod(symbol, rx_bits); | ||
| qam16_demod(symbol, rx_bits, 1.0); | ||
| if (memcmp(tx_bits, rx_bits, 4)) { |
There was a problem hiding this comment.
memcmp(tx_bits, rx_bits, 4) compares only 4 bytes, but tx_bits/rx_bits are int[4] so this only checks the first element on typical platforms. Use sizeof(tx_bits) (or 4 * sizeof(int)) or compare the 4 elements explicitly so the test actually validates all bits.
| if (memcmp(tx_bits, rx_bits, 4)) { | |
| if (memcmp(tx_bits, rx_bits, sizeof(tx_bits))) { |
Update interldpc prototype, hard-decision demod path for bps=4, fix qam16 unit-test comparison width, reduce qam16c2 config duplication, and move symbols_to_llrs scratch buffers off stack.
Set qam16c2 TX BPF prototype to filtP1100S1300 so optional tx_bpf/clip paths follow OFDM mode conventions and don't assert on enable.
Restore STM32 OFDM unittest files close to main and keep only API-alignment changes required by the qam16/psk updates.
cb39cfa to
e2aee0a
Compare
When verbose is enabled, print CRC status and a short hexdump of decoded OFDM data payloads to help diagnose application-level framing/bitstream decode issues.
This PR proposes merging
dr-qam16-cportintomain.Note: the branch currently has merge conflicts with
mainthat need resolution before merge.