Skip to content

Commit

Permalink
Fix integer underflow in sendJVC(). (#401)
Browse files Browse the repository at this point in the history 
Since `sendJVC()` was updated to use `sendGeneric()` it has produced a double
minimum gap at the end of each message, instead of a single one.
This wasn't picked up by our unit-testing.
This tickled a situation where an integer underflow happened when calculating
the required `space()` length, which would cause a delay of approx 1h20m.
Refactored code to eliminate this problem, and a potential similar case
in `sendWhynter()` too.

This is a derivative of the bug/issue #381

This instance was reported in #400, which this patch should fix.
  • Loading branch information
@crankyoldgit
crankyoldgit authored Jan 26, 2018
1 parent fce4ccf commit f6451bf
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 25 deletions.
6 changes: 5 additions & 1 deletion src/ir_JVC.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ void IRsend::sendJVC(uint64_t data, uint16_t nbits, uint16_t repeat) {
data, nbits, 38, true, 0, // Repeats are handles elsewhere.
33);
// Wait till the end of the repeat time window before we send another code.
space(std::max(JVC_RPT_LENGTH - JVC_MIN_GAP - usecs.elapsed(), 0U));
uint32_t elapsed = usecs.elapsed();
// Avoid potential unsigned integer underflow.
// e.g. when elapsed > JVC_RPT_LENGTH.
if (elapsed < JVC_RPT_LENGTH)
space(JVC_RPT_LENGTH - elapsed);
usecs.reset();
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/ir_Whynter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <algorithm>
#include "IRrecv.h"
#include "IRsend.h"
#include "IRtimer.h"
#include "IRutils.h"

// W W H H Y Y N N TTTTT EEEEE RRRRR
Expand Down Expand Up @@ -54,21 +53,19 @@
void IRsend::sendWhynter(uint64_t data, uint16_t nbits, uint16_t repeat) {
// Set IR carrier frequency
enableIROut(38);
IRtimer usecTimer = IRtimer();

for (uint16_t i = 0; i <= repeat; i++) {
usecTimer.reset();
// (Pre-)Header
mark(WHYNTER_BIT_MARK);
space(WHYNTER_ZERO_SPACE);
sendGeneric(WHYNTER_HDR_MARK, WHYNTER_HDR_SPACE,
WHYNTER_BIT_MARK, WHYNTER_ONE_SPACE,
WHYNTER_BIT_MARK, WHYNTER_ZERO_SPACE,
WHYNTER_BIT_MARK, WHYNTER_MIN_GAP,
WHYNTER_MIN_COMMAND_LENGTH - (WHYNTER_BIT_MARK +
WHYNTER_ZERO_SPACE),
data, nbits, 38, true, 0, // Repeats are already handled.
50);
space(std::max(WHYNTER_MIN_COMMAND_LENGTH - WHYNTER_MIN_GAP -
usecTimer.elapsed(), 0U));
}
}
#endif
Expand Down
16 changes: 8 additions & 8 deletions test/ir_JVC_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ TEST(TestSendJVC, SendDataOnly) {
"m8400s4200"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000", irsend.outputStr());
"m525s70875", irsend.outputStr());
}

// Test sending with different repeats.
Expand All @@ -31,22 +31,22 @@ TEST(TestSendJVC, SendWithRepeats) {
"m8400s4200"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000"
"m525s70875"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000", irsend.outputStr());
"m525s70875", irsend.outputStr());
irsend.sendJVC(0xC2B8, JVC_BITS, 2); // 2 repeats.
EXPECT_EQ(
"m8400s4200"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000"
"m525s70875"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000"
"m525s70875"
"m525s1725m525s1725m525s525m525s525m525s525m525s525m525s1725m525s525"
"m525s1725m525s525m525s1725m525s1725m525s1725m525s525m525s525m525s525"
"m525s60000", irsend.outputStr());
"m525s70875", irsend.outputStr());
}

// Test sending an atypical data size.
Expand All @@ -59,7 +59,7 @@ TEST(TestSendJVC, SendUsualSize) {
EXPECT_EQ(
"m8400s4200"
"m525s525m525s525m525s525m525s525m525s525m525s525m525s525m525s525"
"m525s60000", irsend.outputStr());
"m525s70875", irsend.outputStr());

irsend.reset();
irsend.sendJVC(0x1234567890ABCDEF, 64);
Expand All @@ -73,7 +73,7 @@ TEST(TestSendJVC, SendUsualSize) {
"m525s1725m525s525m525s1725m525s525m525s1725m525s525m525s1725m525s1725"
"m525s1725m525s1725m525s525m525s525m525s1725m525s1725m525s525m525s1725"
"m525s1725m525s1725m525s1725m525s525m525s1725m525s1725m525s1725m525s1725"
"m525s60000", irsend.outputStr());
"m525s70875", irsend.outputStr());
}

// Tests for encodeJVC().
Expand Down
22 changes: 11 additions & 11 deletions test/ir_Whynter_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ TEST(TestSendWhynter, SendDataOnly) {
"m750s750m750s750m750s750m750s750m750s750m750s750m750s750m750s750"
"m750s750m750s750m750s750m750s750m750s750m750s750m750s750m750s750"
"m750s750m750s750m750s750m750s750m750s750m750s750m750s750m750s750"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());

irsend.reset();
irsend.sendWhynter(0xFFFFFFFF);
Expand All @@ -29,7 +29,7 @@ TEST(TestSendWhynter, SendDataOnly) {
"m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150"
"m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150"
"m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());

irsend.reset();
irsend.sendWhynter(0x87654321);
Expand All @@ -39,7 +39,7 @@ TEST(TestSendWhynter, SendDataOnly) {
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());
}

// Test sending with different repeats.
Expand All @@ -55,7 +55,7 @@ TEST(TestSendWhynter, SendWithRepeats) {
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());

irsend.reset();
irsend.sendWhynter(0x87654321, WHYNTER_BITS, 1); // 1 repeat.
Expand All @@ -65,33 +65,33 @@ TEST(TestSendWhynter, SendWithRepeats) {
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000"
"m750s106500"
"m750s750m2850s2850"
"m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150m750s2150"
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());
irsend.sendWhynter(0x87654321, WHYNTER_BITS, 2); // 2 repeats.
EXPECT_EQ(
"m750s750m2850s2850"
"m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150m750s2150"
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000"
"m750s106500"
"m750s750m2850s2850"
"m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150m750s2150"
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000"
"m750s106500"
"m750s750m2850s2850"
"m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150m750s2150"
"m750s750m750s2150m750s2150m750s750m750s750m750s2150m750s750m750s2150"
"m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150m750s2150"
"m750s750m750s750m750s2150m750s750m750s750m750s750m750s750m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());
}

// Test sending an atypical data size.
Expand All @@ -104,7 +104,7 @@ TEST(TestSendWhynter, SendUnusualSize) {
EXPECT_EQ(
"m750s750m2850s2850"
"m750s750m750s750m750s750m750s750m750s750m750s750m750s750m750s750"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());

irsend.reset();
irsend.sendWhynter(0x1234567890ABCDEF, 64);
Expand All @@ -118,7 +118,7 @@ TEST(TestSendWhynter, SendUnusualSize) {
"m750s2150m750s750m750s2150m750s750m750s2150m750s750m750s2150m750s2150"
"m750s2150m750s2150m750s750m750s750m750s2150m750s2150m750s750m750s2150"
"m750s2150m750s2150m750s2150m750s750m750s2150m750s2150m750s2150m750s2150"
"m750s108000", irsend.outputStr());
"m750s106500", irsend.outputStr());
}

// Tests for decodeWhynter().
Expand Down

0 comments on commit f6451bf

Please sign in to comment.