Skip to content

Reset parser after inter-byte timeout to prevent truncated frame corruption #32

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.

Sign up for GitHub

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

Open
brocci wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reset parser after inter-byte timeout to prevent truncated frame corruption #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/CMRI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ CMRI::CMRI(unsigned int address, unsigned int input_bits, unsigned int output_bi

// parsing state
,
_mode(PREAMBLE_1), _rx_index(0), _rx_data_len(0), _init_handler(nullptr)
_mode(PREAMBLE_1), _rx_index(0), _rx_data_len(0), _init_handler(nullptr), _last_byte_time_ms(0)

{
// clear to zero
Expand Down Expand Up @@ -164,6 +164,20 @@ void CMRI::transmit()
// Private methods
uint8_t CMRI::_decode(uint8_t c)
{
// Inter-byte timeout: if a gap between bytes exceeds INTER_BYTE_TIMEOUT_MS,
// assume the current frame was truncated and reset the parser to wait for a
// new preamble. This prevents a corrupt frame from consuming subsequent data.
unsigned long now = millis();
if (_mode != PREAMBLE_1 && _mode != PREAMBLE_2 && _mode != PREAMBLE_3)
{
if (now - _last_byte_time_ms > INTER_BYTE_TIMEOUT_MS)
{
_mode = PREAMBLE_1;
_rx_index = 0;
}
}
_last_byte_time_ms = now;

switch (_mode)
{
case PREAMBLE_1:
Expand Down
8 changes: 8 additions & 0 deletions src/CMRI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ class CMRI
// parsing state variables
int _mode;
int _rx_index;
unsigned long _last_byte_time_ms;

#ifndef INTER_BYTE_TIMEOUT_MS
// 1 char time = 11 bits / baud_rate (8N2 framing: 1 start + 8 data + 2 stop)
// default = 5 char times at 9600 bps = 5.73ms
// For other baud rates: (5UL * 11 * 1000) / baud_rate
#define INTER_BYTE_TIMEOUT_MS 6
#endif

uint8_t _decode(uint8_t c); // process one character received from serial port
};
Expand Down
10 changes: 10 additions & 0 deletions test/mock/Arduino.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ inline void delayMicroseconds(unsigned int)
{
}

extern unsigned long _mock_millis;
inline unsigned long millis()
{
return _mock_millis;
}
inline void mock_advance_millis(unsigned long ms)
{
_mock_millis += ms;
}

// The default argument of the CMRI constructor references Serial; the test
// translation unit defines it.
extern Stream Serial;
Expand Down
75 changes: 75 additions & 0 deletions test/test_cmri/test_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
// which always pass an explicit Stream, but needed to satisfy the symbol).
Stream Serial;

// Definition of the mock millis variable (declared extern in mock/Arduino.h).
unsigned long _mock_millis = 0;

void setUp(void)
{
_mock_millis = 0;
}

void tearDown(void)
Expand Down Expand Up @@ -194,6 +198,75 @@ void test_preamble_resync_after_garbage(void)
TEST_ASSERT_EQUAL_UINT8(CMRI::GET, s.tx[4]);
}

// A truncated frame does not corrupt the next frame after a timeout.
void test_truncated_frame_does_not_corrupt_next(void)
{
Stream s;
CMRI cmri(0, 24, 48, s);

// Feed a partial SET without ETX (truncated)
s.feed(0xFF);
s.feed(0xFF);
s.feed(CMRI::STX);
s.feed('A' + 0);
s.feed(CMRI::SET);
s.feed(0xAA);
s.feed(0xBB);
s.feed(0xCC);

// Process the partial frame -- should not return true
TEST_ASSERT_FALSE(cmri.process());

// Advance time past the inter-byte timeout
mock_advance_millis(10);

// Now send a complete SET (skip 0x03 -- protocol byte)
uint8_t full[6] = {0x01, 0x02, 0x04, 0x05, 0x06, 0x07};
feed_packet(s, 0, CMRI::SET, full, 6);

TEST_ASSERT_TRUE(cmri.process());

// Only the second SET's data should be in the output buffer
TEST_ASSERT_EQUAL_UINT8(0x01, cmri.get_byte(0));
TEST_ASSERT_EQUAL_UINT8(0x02, cmri.get_byte(1));
TEST_ASSERT_EQUAL_UINT8(0x04, cmri.get_byte(2));
TEST_ASSERT_EQUAL_UINT8(0x05, cmri.get_byte(3));
TEST_ASSERT_EQUAL_UINT8(0x06, cmri.get_byte(4));
TEST_ASSERT_EQUAL_UINT8(0x07, cmri.get_byte(5));
}

// A DLE at the end of a truncated frame does not corrupt the next frame.
void test_dle_at_end_of_truncated_frame(void)
{
Stream s;
CMRI cmri(0, 24, 48, s);

// Feed a partial SET ending with ESC byte (no following byte)
s.feed(0xFF);
s.feed(0xFF);
s.feed(CMRI::STX);
s.feed('A' + 0);
s.feed(CMRI::SET);
s.feed(CMRI::ESC);

// Process the partial frame -- should not return true
TEST_ASSERT_FALSE(cmri.process());

// Advance time past the inter-byte timeout
mock_advance_millis(10);

// Now send a complete SET (skip 0x03 -- protocol byte)
uint8_t full[6] = {0x01, 0x02, 0x04, 0x05, 0x06, 0x07};
feed_packet(s, 0, CMRI::SET, full, 6);

TEST_ASSERT_TRUE(cmri.process());

TEST_ASSERT_EQUAL_UINT8(0x01, cmri.get_byte(0));
TEST_ASSERT_EQUAL_UINT8(0x02, cmri.get_byte(1));
TEST_ASSERT_EQUAL_UINT8(0x04, cmri.get_byte(2));
TEST_ASSERT_EQUAL_UINT8(0x05, cmri.get_byte(3));
}

int main(int, char **)
{
UNITY_BEGIN();
Expand All @@ -206,5 +279,7 @@ int main(int, char **)
RUN_TEST(test_address_filtering);
RUN_TEST(test_transmit_escapes_control_bytes);
RUN_TEST(test_preamble_resync_after_garbage);
RUN_TEST(test_truncated_frame_does_not_corrupt_next);
RUN_TEST(test_dle_at_end_of_truncated_frame);
return UNITY_END();
}