From 257a15130ff2a3386984ea96405ca50707c23237 Mon Sep 17 00:00:00 2001 From: Philipp Henkel Date: Sun, 26 Mar 2017 22:32:55 +0200 Subject: [PATCH] Fix calculation of pause length in LEGO PF protocol Fix #384 Integer overflow in LEGO Power Functions affects pause between messages Is rebased version of PR #385 --- .../LegoPowerFunctionsTests.ino | 18 +++---- ir_Lego_PF_BitStreamEncoder.h | 54 +++++++++---------- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/examples/LegoPowerFunctionsTests/LegoPowerFunctionsTests.ino b/examples/LegoPowerFunctionsTests/LegoPowerFunctionsTests.ino index 6a2bda5..65760e2 100644 --- a/examples/LegoPowerFunctionsTests/LegoPowerFunctionsTests.ino +++ b/examples/LegoPowerFunctionsTests/LegoPowerFunctionsTests.ino @@ -1,6 +1,6 @@ /* * LegoPowerFunctionsTest: LEGO Power Functions Tests - * Copyright (c) 2016 Philipp Henkel + * Copyright (c) 2016, 2017 Philipp Henkel */ #include @@ -78,14 +78,14 @@ void testMessageBitCount(LegoPfBitStreamEncoder& bitStreamEncoder) { logTestResult(bitCount == 18); } -boolean check(LegoPfBitStreamEncoder& bitStreamEncoder, int markDuration, int pauseDuration) { +boolean check(LegoPfBitStreamEncoder& bitStreamEncoder, unsigned long markDuration, unsigned long pauseDuration) { bool result = true; result = result && bitStreamEncoder.getMarkDuration() == markDuration; result = result && bitStreamEncoder.getPauseDuration() == pauseDuration; return result; } -boolean checkNext(LegoPfBitStreamEncoder& bitStreamEncoder, int markDuration, int pauseDuration) { +boolean checkNext(LegoPfBitStreamEncoder& bitStreamEncoder, unsigned long markDuration, unsigned long pauseDuration) { bool result = bitStreamEncoder.next(); result = result && check(bitStreamEncoder, markDuration, pauseDuration); return result; @@ -129,16 +129,16 @@ void testMessage407Repeated(LegoPfBitStreamEncoder& bitStreamEncoder) { bool result = true; result = result && check(bitStreamEncoder, 158, 1026); result = result && checkDataBitsOfMessage407(bitStreamEncoder); - result = result && checkNext(bitStreamEncoder, 158, 1026 + 5 * 16000 - 10844); + result = result && checkNext(bitStreamEncoder, 158, 1026L + 5L * 16000L - 10844L); result = result && checkNext(bitStreamEncoder, 158, 1026); result = result && checkDataBitsOfMessage407(bitStreamEncoder); - result = result && checkNext(bitStreamEncoder, 158, 1026 + 5 * 16000 - 10844); + result = result && checkNext(bitStreamEncoder, 158, 1026L + 5L * 16000L - 10844L); result = result && checkNext(bitStreamEncoder, 158, 1026); result = result && checkDataBitsOfMessage407(bitStreamEncoder); - result = result && checkNext(bitStreamEncoder, 158, 1026 + 8 * 16000 - 10844); + result = result && checkNext(bitStreamEncoder, 158, 1026L + 8L * 16000L - 10844L); result = result && checkNext(bitStreamEncoder, 158, 1026); result = result && checkDataBitsOfMessage407(bitStreamEncoder); - result = result && checkNext(bitStreamEncoder, 158, 1026 + 8 * 16000 - 10844); + result = result && checkNext(bitStreamEncoder, 158, 1026L + 8L * 16000L - 10844L); result = result && checkNext(bitStreamEncoder, 158, 1026); result = result && checkDataBitsOfMessage407(bitStreamEncoder); result = result && checkNext(bitStreamEncoder, 158, 1026); @@ -191,7 +191,3 @@ void testGetMessageLengthAllLow(LegoPfBitStreamEncoder& bitStreamEncoder) { bitStreamEncoder.reset(0x0, false); logTestResult(bitStreamEncoder.getMessageLength() == 9104); } - - - - diff --git a/ir_Lego_PF_BitStreamEncoder.h b/ir_Lego_PF_BitStreamEncoder.h index 7689cde..e5a3d66 100644 --- a/ir_Lego_PF_BitStreamEncoder.h +++ b/ir_Lego_PF_BitStreamEncoder.h @@ -4,7 +4,7 @@ // L E E O O // L EEEE E EEE O O // L E E E O O LEGO Power Functions -// LLLLLL EEEEEE EEEE OOOO Copyright (c) 2016 Philipp Henkel +// LLLLLL EEEEEE EEEE OOOO Copyright (c) 2016, 2017 Philipp Henkel //============================================================================== //+============================================================================= @@ -14,25 +14,25 @@ class LegoPfBitStreamEncoder { private: uint16_t data; bool repeatMessage; - int messageBitIdx; - int repeatCount; - int messageLength; - - // HIGH data bit = IR mark + high pause - // LOW data bit = IR mark + low pause - static const int LOW_BIT_DURATION = 421; - static const int HIGH_BIT_DURATION = 711; - static const int START_BIT_DURATION = 1184; - static const int STOP_BIT_DURATION = 1184; - static const int IR_MARK_DURATION = 158; - static const int HIGH_PAUSE_DURATION = HIGH_BIT_DURATION - IR_MARK_DURATION; - static const int LOW_PAUSE_DURATION = LOW_BIT_DURATION - IR_MARK_DURATION; - static const int START_PAUSE_DURATION = START_BIT_DURATION - IR_MARK_DURATION; - static const int STOP_PAUSE_DURATION = STOP_BIT_DURATION - IR_MARK_DURATION; - static const int MESSAGE_BITS = 18; - static const int MAX_MESSAGE_LENGTH = 16000; + uint8_t messageBitIdx; + uint8_t repeatCount; + uint16_t messageLength; public: + // HIGH data bit = IR mark + high pause + // LOW data bit = IR mark + low pause + static const uint16_t LOW_BIT_DURATION = 421; + static const uint16_t HIGH_BIT_DURATION = 711; + static const uint16_t START_BIT_DURATION = 1184; + static const uint16_t STOP_BIT_DURATION = 1184; + static const uint8_t IR_MARK_DURATION = 158; + static const uint16_t HIGH_PAUSE_DURATION = HIGH_BIT_DURATION - IR_MARK_DURATION; + static const uint16_t LOW_PAUSE_DURATION = LOW_BIT_DURATION - IR_MARK_DURATION; + static const uint16_t START_PAUSE_DURATION = START_BIT_DURATION - IR_MARK_DURATION; + static const uint16_t STOP_PAUSE_DURATION = STOP_BIT_DURATION - IR_MARK_DURATION; + static const uint8_t MESSAGE_BITS = 18; + static const uint16_t MAX_MESSAGE_LENGTH = 16000; + void reset(uint16_t data, bool repeatMessage) { this->data = data; this->repeatMessage = repeatMessage; @@ -43,9 +43,9 @@ class LegoPfBitStreamEncoder { int getChannelId() const { return 1 + ((data >> 12) & 0x3); } - int getMessageLength() const { + uint16_t getMessageLength() const { // Sum up all marks - int length = MESSAGE_BITS * IR_MARK_DURATION; + uint16_t length = MESSAGE_BITS * IR_MARK_DURATION; // Sum up all pauses length += START_PAUSE_DURATION; @@ -75,9 +75,9 @@ class LegoPfBitStreamEncoder { } } - int getMarkDuration() const { return IR_MARK_DURATION; } + uint8_t getMarkDuration() const { return IR_MARK_DURATION; } - int getPauseDuration() const { + uint32_t getPauseDuration() const { if (messageBitIdx == 0) return START_PAUSE_DURATION; else if (messageBitIdx < MESSAGE_BITS - 1) { @@ -88,13 +88,13 @@ class LegoPfBitStreamEncoder { } private: - int getDataBitPause() const { + uint16_t getDataBitPause() const { const int pos = MESSAGE_BITS - 2 - messageBitIdx; const bool isHigh = data & (1 << pos); return isHigh ? HIGH_PAUSE_DURATION : LOW_PAUSE_DURATION; } - int getStopPause() const { + uint32_t getStopPause() const { if (repeatMessage) { return getRepeatStopPause(); } else { @@ -102,12 +102,12 @@ class LegoPfBitStreamEncoder { } } - int getRepeatStopPause() const { + uint32_t getRepeatStopPause() const { if (repeatCount == 0 || repeatCount == 1) { - return STOP_PAUSE_DURATION + 5 * MAX_MESSAGE_LENGTH - messageLength; + return STOP_PAUSE_DURATION + (uint32_t)5 * MAX_MESSAGE_LENGTH - messageLength; } else if (repeatCount == 2 || repeatCount == 3) { return STOP_PAUSE_DURATION - + (6 + 2 * getChannelId()) * MAX_MESSAGE_LENGTH - messageLength; + + (uint32_t)(6 + 2 * getChannelId()) * MAX_MESSAGE_LENGTH - messageLength; } else { return STOP_PAUSE_DURATION; }