Skip to content

Commit

Permalink
Eliminates a copy in the DCC decoder. (#547)
Browse files Browse the repository at this point in the history 
* Switches the DCC decoder to use zero-copy buffering.

* Adds yield when the next packet shows up.

* Fix style to be simpler to read.

* Fixes bugs around packet length.
  • Loading branch information
@balazsracz
balazsracz authored Jul 10, 2021
1 parent d551997 commit 9ba9ae1
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 69 deletions.
121 changes: 85 additions & 36 deletions src/dcc/Receiver.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

#include "freertos/can_ioctl.h"
#include "freertos_drivers/common/SimpleLog.hxx"
#include "dcc/packet.h"

// If defined, collects samples of timing and state into a ring buffer.
//#define DCC_DECODER_DEBUG
Expand Down Expand Up @@ -89,6 +90,27 @@ public:
return parseState_;
}

/// Sets where to write the decoded DCC data to. If this function is not
/// called before a packet preamble starts, the packet will be discarded.
/// @param pkt where to store the incoming payload bytes. This packet will
/// be reset to an empty packet.
void set_packet(DCCPacket *pkt)
{
pkt_ = pkt;
if (pkt_)
{
clear_packet();
}
}

/// Retrieves the last applied DCC packet pointer.
/// @return DCC packet, or nullptr if there was no DCC packet pointer
/// assigned yet.
DCCPacket *pkt()
{
return pkt_;
};

/// Call this function for each time the polarity of the signal changes.
/// @param value is the number of clock cycles since the last polarity
/// change.
Expand All @@ -110,12 +132,14 @@ public:
parseState_ = DCC_PREAMBLE;
return;
}
if (timings_[MM_PREAMBLE].match(value))
if (timings_[MM_PREAMBLE].match(value) && pkt_)
{
parseCount_ = 1 << 2;
ofs_ = 0;
data_[ofs_] = 0;
pkt_->dlc = 0;
pkt_->payload[0] = 0;
pkt_->packet_header.is_marklin = 1;
parseState_ = MM_DATA;
havePacket_ = true;
}
break;
}
Expand All @@ -139,8 +163,18 @@ public:
{
parseState_ = DCC_DATA;
parseCount_ = 1 << 7;
ofs_ = 0;
data_[ofs_] = 0;
if (pkt_)
{
havePacket_ = true;
clear_packet();
pkt_->dlc = 0;
pkt_->payload[0] = 0;
pkt_->packet_header.skip_ec = 1;
}
else
{
havePacket_ = false;
}
return;
}
break;
Expand All @@ -165,14 +199,21 @@ public:
{
if (parseCount_)
{
data_[ofs_] |= parseCount_;
if (havePacket_)
{
pkt_->payload[pkt_->dlc] |= parseCount_;
}
parseCount_ >>= 1;
parseState_ = DCC_DATA;
return;
}
else
{
// end of packet 1 bit.
if (havePacket_)
{
pkt_->dlc++;
}
parseState_ = DCC_MAYBE_CUTOUT;
return;
}
Expand All @@ -192,9 +233,18 @@ public:
else
{
// end of byte zero bit. Packet is not finished yet.
ofs_++;
HASSERT(ofs_ < sizeof(data_));
data_[ofs_] = 0;
if (havePacket_)
{
pkt_->dlc++;
if (pkt_->dlc >= DCC_PACKET_MAX_PAYLOAD)
{
havePacket_ = false;
}
else
{
pkt_->payload[pkt_->dlc] = 0;
}
}
parseCount_ = 1 << 7;
}
parseState_ = DCC_DATA;
Expand Down Expand Up @@ -239,16 +289,16 @@ public:
parseCount_ >>= 1;
if (!parseCount_)
{
if (ofs_ == 2)
if (pkt_->dlc == 2)
{
parseState_ = MM_PACKET_FINISHED;
return;
}
else
{
ofs_++;
pkt_->dlc++;
parseCount_ = 1 << 7;
data_[ofs_] = 0;
pkt_->payload[pkt_->dlc] = 0;
}
}
parseState_ = MM_DATA;
Expand All @@ -260,20 +310,20 @@ public:
{
if (timings_[MM_LONG].match(value))
{
data_[ofs_] |= parseCount_;
pkt_->payload[pkt_->dlc] |= parseCount_;
parseCount_ >>= 1;
if (!parseCount_)
{
if (ofs_ == 2)
if (pkt_->dlc == 2)
{
parseState_ = MM_PACKET_FINISHED;
return;
}
else
{
ofs_++;
pkt_->dlc++;
parseCount_ = 1 << 7;
data_[ofs_] = 0;
pkt_->payload[pkt_->dlc] = 0;
}
}
parseState_ = MM_DATA;
Expand All @@ -293,25 +343,23 @@ public:
(parseState_ == DCC_DATA_ONE); // one bit comes
}

/// Returns the number of payload bytes in the current packet.
uint8_t packet_length()
{
return ofs_ + 1;
}

/// Returns the current packet payload buffer. The buffer gets invalidated
/// at the next call to process_data.
const uint8_t *packet_data()
{
return data_;
}

private:
/// Counter that works through bit patterns.
uint32_t parseCount_ = 0;
/// State machine for parsing.
State parseState_ = UNKNOWN;
// Payload of current packet.
uint8_t data_[6];
uint8_t ofs_; // offset inside data_;
/// True if we have storage in the right time for the current packet.
bool havePacket_;
/// Storage for the current packet.
DCCPacket* pkt_ = nullptr;

/// Sets the input DCC packet to empty.
void clear_packet()
{
pkt_->header_raw_data = 0;
pkt_->dlc = 0;
pkt_->feedback_key = 0;
}

/// Represents the timing of a half-wave of the digital track signal.
struct Timing
Expand Down Expand Up @@ -377,6 +425,7 @@ public:
: StateFlowBase(s)
{
fd_ = ::open(dev, O_RDONLY | O_NONBLOCK);
decoder_.set_packet(&pkt_);
start_flow(STATE(register_and_sleep));
}

Expand All @@ -401,13 +450,11 @@ private:
decoder_.process_data(value);
if (decoder_.state() == DccDecoder::DCC_PACKET_FINISHED)
{
dcc_packet_finished(
decoder_.packet_data(), decoder_.packet_length());
dcc_packet_finished(pkt_.payload, pkt_.dlc);
}
else if (decoder_.state() == DccDecoder::MM_PACKET_FINISHED)
{
mm_packet_finished(
decoder_.packet_data(), decoder_.packet_length());
mm_packet_finished(pkt_.payload, pkt_.dlc);
}

static uint8_t x = 0;
Expand All @@ -426,6 +473,8 @@ private:
uint32_t lastValue_ = 0;

protected:
/// Packet buffer.
DCCPacket pkt_;
/// State machine that does the DCC decoding. We have 1 usec per tick, as
/// these are the numbers we receive from the driver.
DccDecoder decoder_ {1};
Expand Down
58 changes: 25 additions & 33 deletions src/freertos_drivers/common/DccDecoder.hxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ private:
{
portENTER_CRITICAL();
unsigned copied = inputData_->get((input_data_type *)buf, max);
if (!nextPacketData_ && inputData_->space())
if (!decoder_.pkt() && inputData_->space())
{
inputData_->data_write_pointer(&nextPacketData_);
DCCPacket* next;
inputData_->data_write_pointer(&next);
decoder_.set_packet(next);
}
portEXIT_CRITICAL();
if (copied > 0)
Expand Down Expand Up @@ -190,7 +192,6 @@ private:
typedef DCCPacket input_data_type;
DeviceBuffer<DCCPacket> *inputData_ {
DeviceBuffer<DCCPacket>::create(Module::Q_SIZE)};
DCCPacket* nextPacketData_{nullptr};
bool nextPacketFilled_{false};
/// Holds the value of the free running timer at the time we captured the
/// previous edge.
Expand All @@ -212,7 +213,7 @@ private:
/// Which window of the cutout we are in.
uint32_t cutoutState_;
/// How many times did we lose a DCC packet due to no buffer available.
uint32_t overflowCount_ {0};
//uint32_t overflowCount_ {0};

/// notified for cutout events.
RailcomDriver *railcomDriver_;
Expand Down Expand Up @@ -253,12 +254,11 @@ template <class Module> void DccDecoder<Module>::enable()
lastTimerValue_ = Module::TIMER_MAX_VALUE;
nextSample_ = lastTimerValue_ - Module::SAMPLE_PERIOD_CLOCKS;

if (!nextPacketData_)
if (!decoder_.pkt() && inputData_->space())
{
if (inputData_->space())
{
inputData_->data_write_pointer(&nextPacketData_);
}
DCCPacket *next;
inputData_->data_write_pointer(&next);
decoder_.set_packet(next);
}
}

Expand Down Expand Up @@ -317,6 +317,11 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
Module::set_cap_timer_delay_usec(RAILCOM_CUTOUT_PRE);
inCutout_ = true;
cutoutState_ = 0;
if (decoder_.pkt())
{
nextPacketFilled_ = true;
}
Module::trigger_os_interrupt();
}
else if (decoder_.state() == dcc::DccDecoder::DCC_CUTOUT)
{
Expand All @@ -333,24 +338,6 @@ __attribute__((optimize("-O3"))) void DccDecoder<Module>::interrupt_handler()
Module::dcc_packet_finished_hook();
prepCutout_ = false;
cutout_just_finished = true;
// Record packet to send back to userspace
if (nextPacketData_)
{
nextPacketData_->header_raw_data = 0;
nextPacketData_->packet_header.skip_ec = 1;
nextPacketData_->dlc = decoder_.packet_length();
memcpy(nextPacketData_->payload, decoder_.packet_data(),
decoder_.packet_length());
nextPacketData_ = nullptr;
nextPacketFilled_ = true;

Module::trigger_os_interrupt();
}
else
{
// Lost DCC packet.
overflowCount_++;
}
Debug::DccPacketFinishedHook::set(false);
}
lastTimerValue_ = raw_new_value;
Expand Down Expand Up @@ -406,15 +393,20 @@ DccDecoder<Module>::rcom_interrupt_handler()
template <class Module>
__attribute__((optimize("-O3"))) void DccDecoder<Module>::os_interrupt_handler()
{
if (nextPacketFilled_) {
unsigned woken = 0;
if (nextPacketFilled_)
{
inputData_->advance(1);
nextPacketFilled_ = false;
inputData_->signal_condition_from_isr();
woken = 1;
decoder_.set_packet(nullptr);
}
if (!nextPacketData_) {
if (inputData_->space())
{
inputData_->data_write_pointer(&nextPacketData_);
}
if (!decoder_.pkt() && inputData_->space())
{
DCCPacket *next;
inputData_->data_write_pointer(&next);
decoder_.set_packet(next);
}
portYIELD_FROM_ISR(woken);
}

0 comments on commit 9ba9ae1

Please sign in to comment.