cherrypick changes for V.24 fixes from tsawyer:dvmhost:codex/fix-v24-r05a05-prready;

pull/117/head
Bryan Biedenkapp 3 weeks ago
parent d05a593b36
commit 8c21388603

@ -4,7 +4,8 @@
* GPLv2 Open Source. Use is subject to license terms. * GPLv2 Open Source. Use is subject to license terms.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* Copyright (C) 2017-2024 Bryan Biedenkapp, N2PLL * Copyright (C) 2017-2026 Bryan Biedenkapp, N2PLL
* Copyright (C) 2026 Timothy Sawyer, WD6AWP
* *
*/ */
#include "Defines.h" #include "Defines.h"
@ -255,6 +256,14 @@ void* Host::threadP25Writer(void* arg)
if (nextLen > 0U) { if (nextLen > 0U) {
bool ret = host->m_modem->hasP25Space(nextLen); bool ret = host->m_modem->hasP25Space(nextLen);
// are we connected to a V.24 device? if so, always force
// allow the write, V.24 modem status space can lag at call start; do not block
// initial Net->RF voice frames on stale p25Space accounting
if (host->m_modem->isV24Connected()) {
ret = true;
}
if (ret) { if (ret) {
bool imm = false; bool imm = false;
uint32_t len = host->m_p25->getFrame(data, &imm); uint32_t len = host->m_p25->getFrame(data, &imm);
@ -267,7 +276,7 @@ void* Host::threadP25Writer(void* arg)
// if the state is P25; write P25 frame data // if the state is P25; write P25 frame data
if (host->m_state == STATE_P25) { if (host->m_state == STATE_P25) {
host->m_modem->writeP25Frame(data, len); host->m_modem->writeP25Frame(data, len, imm);
afterWriteCallback(); afterWriteCallback();

@ -5,8 +5,9 @@
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* Copyright (C) 2011-2021 Jonathan Naylor, G4KLX * Copyright (C) 2011-2021 Jonathan Naylor, G4KLX
* Copyright (C) 2017-2024 Bryan Biedenkapp, N2PLL * Copyright (C) 2017-2026 Bryan Biedenkapp, N2PLL
* Copyright (C) 2021 Nat Moore * Copyright (C) 2021 Nat Moore
* Copyright (C) 2026 Timothy Sawyer, WD6AWP
* *
*/ */
#include "Defines.h" #include "Defines.h"
@ -125,7 +126,7 @@ Modem::Modem(port::IModemPort* port, bool duplex, bool rxInvert, bool txInvert,
m_nxdnFifoLength(NXDN_TX_BUFFER_LEN), m_nxdnFifoLength(NXDN_TX_BUFFER_LEN),
m_adcOverFlowCount(0U), m_adcOverFlowCount(0U),
m_dacOverFlowCount(0U), m_dacOverFlowCount(0U),
m_v24Connected(true), m_v24Connected(false),
m_modemState(STATE_IDLE), m_modemState(STATE_IDLE),
m_buffer(nullptr), m_buffer(nullptr),
m_length(0U), m_length(0U),
@ -754,7 +755,7 @@ void Modem::clock(uint32_t ms)
// flag indicating if free space is being reported in 16-byte blocks instead of LDUs // flag indicating if free space is being reported in 16-byte blocks instead of LDUs
bool spaceInBlocks = (m_buffer[3U] & 0x80U) == 0x80U; bool spaceInBlocks = (m_buffer[3U] & 0x80U) == 0x80U;
m_v24Connected = true; m_v24Connected = false;
m_modemState = (DVM_STATE)m_buffer[4U]; m_modemState = (DVM_STATE)m_buffer[4U];
m_tx = (m_buffer[5U] & 0x01U) == 0x01U; m_tx = (m_buffer[5U] & 0x01U) == 0x01U;
@ -1517,7 +1518,9 @@ bool Modem::writeP25Frame(const uint8_t* data, uint32_t length, bool imm)
uint32_t len = length + 2U; uint32_t len = length + 2U;
// write or buffer P25 data to air interface // write or buffer P25 data to air interface
if (m_p25Space >= length) { // for V.24/DFSI, the modem status space value may lag call startup; do
// not block initial Net->RF frames solely on stale p25Space
if (m_p25Space >= length || isV24Connected()) {
if (m_debug) if (m_debug)
LogDebugEx(LOG_MODEM, "Modem::writeP25Frame()", "immediate write (len %u)", length); LogDebugEx(LOG_MODEM, "Modem::writeP25Frame()", "immediate write (len %u)", length);
if (m_trace) if (m_trace)
@ -1529,6 +1532,17 @@ bool Modem::writeP25Frame(const uint8_t* data, uint32_t length, bool imm)
return false; return false;
} }
/*
** bryanb: this change is from Tim's fork, but it is a bit concerning, because we're not adjusting
** available modem space appropriately, if there isn't enough space for the frame being written in the first
** place -- for now i've commented this out but included it, reasonably speaking m_p25Space doesn't even
** matter because writeSerial() in ModemV24 doesn't even check this before writing to the port
*/
/*
if (m_p25Space >= length) {
m_p25Space -= length;
}
*/
m_p25Space -= length; m_p25Space -= length;
} }
else { else {

@ -4,8 +4,9 @@
* GPLv2 Open Source. Use is subject to license terms. * GPLv2 Open Source. Use is subject to license terms.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* Copyright (C) 2024-2025 Bryan Biedenkapp, N2PLL * Copyright (C) 2024-2026 Bryan Biedenkapp, N2PLL
* Copyright (C) 2024 Patrick McDonnell, W3AXL * Copyright (C) 2024 Patrick McDonnell, W3AXL
* Copyright (C) 2026 Timothy Sawyer, WD6AWP
* *
*/ */
#include "Defines.h" #include "Defines.h"
@ -394,6 +395,14 @@ void ModemV24::clock(uint32_t ms)
int len = 0; int len = 0;
// write anything waiting to the serial port // write anything waiting to the serial port
/*
** bryanb: Tim's fork changes the order of this, however, this is a terrible idea becuase it will ultimately break
** the intended purpose of this code which is to allow prioritized frames, reasonably speaking this code should
** not cause problems for timing -- but we should keep an eye on this (if its problematic for some reason,
** we can consider perhaps adding a flag that disables the immediate queue, forcing all packets in to the normal
** queue)
*/
if (!m_txImmP25Queue.isEmpty()) if (!m_txImmP25Queue.isEmpty())
len = writeSerial(&m_txImmP25Queue); len = writeSerial(&m_txImmP25Queue);
else else
@ -477,6 +486,8 @@ int ModemV24::writeSerial(RingBuffer<uint8_t>* queue)
* | Length | Tag | int64_t timestamp in ms | data | * | Length | Tag | int64_t timestamp in ms | data |
*/ */
std::lock_guard<std::mutex> lock(m_txP25QueueLock);
// check empty // check empty
if (queue->isEmpty()) if (queue->isEmpty())
return 0U; return 0U;
@ -486,24 +497,19 @@ int ModemV24::writeSerial(RingBuffer<uint8_t>* queue)
::memset(length, 0x00U, 2U); ::memset(length, 0x00U, 2U);
queue->peek(length, 2U); queue->peek(length, 2U);
// convert length byets to int // convert length bytes to int
uint16_t len = 0U; uint16_t len = 0U;
len = (length[0U] << 8) + length[1U]; len = (length[0U] << 8) + length[1U];
// this ensures we never get in a situation where we have length & type bytes stuck in the queue by themselves // this ensures we never get in a situation where we have length bytes stuck in the queue by themselves
if (queue->dataSize() == 2U && len > queue->dataSize()) { if (queue->dataSize() == 2U && len > queue->dataSize()) {
queue->get(length, 2U); // ensure we pop bytes off queue->get(length, 2U); // ensure we pop bytes off
return 0U; return 0U;
} }
// check available modem space
if (m_p25Space < len)
return 0U;
std::lock_guard<std::mutex> lock(m_txP25QueueLock);
// get current timestamp // get current timestamp
int64_t now = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count(); int64_t now = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now().time_since_epoch()).count();
int64_t ts = 0L;
// peek the timestamp to see if we should wait // peek the timestamp to see if we should wait
if (queue->dataSize() >= 11U) { if (queue->dataSize() >= 11U) {
@ -512,7 +518,6 @@ int ModemV24::writeSerial(RingBuffer<uint8_t>* queue)
queue->peek(lengthTagTs, 11U); queue->peek(lengthTagTs, 11U);
// get the timestamp // get the timestamp
int64_t ts;
assert(sizeof ts == 8); assert(sizeof ts == 8);
::memcpy(&ts, lengthTagTs + 3U, 8U); ::memcpy(&ts, lengthTagTs + 3U, 8U);
@ -524,23 +529,34 @@ int ModemV24::writeSerial(RingBuffer<uint8_t>* queue)
// check if we have enough data to get everything - len + 2U (length bytes) + 1U (tag) + 8U (timestamp) // check if we have enough data to get everything - len + 2U (length bytes) + 1U (tag) + 8U (timestamp)
if (queue->dataSize() >= len + 11U) { if (queue->dataSize() >= len + 11U) {
// Get the length, tag and timestamp // peek the full entry so we can keep it queued if a write fails
uint8_t lengthTagTs[11U]; DECLARE_UINT8_ARRAY(entry, len + 11U);
queue->get(lengthTagTs, 11U); queue->peek(entry, len + 11U);
// Get the actual data // get the length, tag, timestamp and payload pointers from peeked data
DECLARE_UINT8_ARRAY(buffer, len); uint8_t* lengthTagTs = entry;
queue->get(buffer, len); uint8_t* buffer = entry + 11U;
// Sanity check on data tag // sanity check on data tag
uint8_t tag = lengthTagTs[2U]; uint8_t tag = lengthTagTs[2U];
if (tag != TAG_DATA) { if (tag != TAG_DATA) {
LogError(LOG_MODEM, "Got unexpected data tag from TX P25 ringbuffer! %02X", tag); LogError(LOG_MODEM, "Got unexpected data tag from TX P25 ringbuffer! %02X", tag);
// drop malformed entry so we can recover queue alignment
DECLARE_UINT8_ARRAY(discard, len + 11U);
queue->get(discard, len + 11U);
return 0U; return 0U;
} }
// we already checked the timestamp above, so we just get the data and write it // we already checked the timestamp above, so we just write it
return m_port->write(buffer, len); int ret = m_port->write(buffer, len);
if (ret > 0) {
// only remove an entry once it was written successfully
DECLARE_UINT8_ARRAY(discard, len + 11U);
queue->get(discard, len + 11U);
}
return ret;
} }
return 0U; return 0U;
@ -2363,12 +2379,19 @@ bool ModemV24::queueP25Frame(uint8_t* data, uint16_t len, SERIAL_TX_TYPE msgType
std::lock_guard<std::mutex> lock(m_txP25QueueLock); std::lock_guard<std::mutex> lock(m_txP25QueueLock);
// check available ringbuffer space // check available ringbuffer space
// keep one byte of headroom to avoid the ring buffer "full == empty"
// ambiguity (iPtr == oPtr) when a write would consume exactly all free space
uint32_t needed = len + 11U;
if (imm) { if (imm) {
if (m_txImmP25Queue.freeSpace() < (len + 11U)) uint32_t free = m_txImmP25Queue.freeSpace();
if (free <= needed) {
return false; return false;
}
} else { } else {
if (m_txP25Queue.freeSpace() < (len + 11U)) uint32_t free = m_txP25Queue.freeSpace();
if (free <= needed) {
return false; return false;
}
} }
// convert 16-bit length to 2 bytes // convert 16-bit length to 2 bytes
@ -2429,6 +2452,10 @@ void ModemV24::startOfStreamV24(const p25::lc::LC& control)
{ {
m_txCallInProgress = true; m_txCallInProgress = true;
// start each Net->RF stream with a fresh scheduler epoch so a new call
// doesn't inherit a future-biased timestamp from the previous call
m_lastP25Tx = 0U;
MotStartOfStream start = MotStartOfStream(); MotStartOfStream start = MotStartOfStream();
start.setOpcode(m_rtrt ? MotStartStreamOpcode::TRANSMIT : MotStartStreamOpcode::RECEIVE); start.setOpcode(m_rtrt ? MotStartStreamOpcode::TRANSMIT : MotStartStreamOpcode::RECEIVE);
start.setParam1(DSFI_MOT_ICW_PARM_PAYLOAD); start.setParam1(DSFI_MOT_ICW_PARM_PAYLOAD);
@ -2546,6 +2573,10 @@ void ModemV24::startOfStreamTIA(const p25::lc::LC& control)
m_txCallInProgress = true; m_txCallInProgress = true;
m_superFrameCnt = 1U; m_superFrameCnt = 1U;
// start each Net->RF stream with a fresh scheduler epoch so a new call
// doesn't inherit a future-biased timestamp from the previous call
m_lastP25Tx = 0U;
p25::lc::LC lc = p25::lc::LC(control); p25::lc::LC lc = p25::lc::LC(control);
uint16_t length = 0U; uint16_t length = 0U;
@ -2806,7 +2837,8 @@ void ModemV24::convertFromAirV24(uint8_t* data, uint32_t length, bool imm)
break; break;
case DUID::TDU: case DUID::TDU:
endOfStreamV24(); // this may incorrectly sent STOP ICW's with the VOICE payload, but it's better than nothing for now if (m_txCallInProgress)
endOfStreamV24();
break; break;
case DUID::TDULC: case DUID::TDULC:

Loading…
Cancel
Save

Powered by TurnKey Linux.