Skip to content

Commit 1aa31ea

Browse files
authored
LG: Fix uninitialised and never-updated Swing(H) previous state (#2287)
* LG: Fix uninitialised and never-updated Swing(H) previous state `updateSwingPrev()` copies `_swingv` and `_vaneswingv[]` across to their `_prev` counterparts, but never `_swingh`. As `updateSwingPrev()` is also what `stateReset()` uses to prime those values, `_swingh_prev` is never initialised at all, and never updated after a send either. That has two consequences for the AKB73757604 model, which is the only one that consults it in `send()`: * The first `send()` reads an uninitialised bool. UBSAN agrees: `ir_LG.cpp:280:24: runtime error: load of value 74, which is not a valid value for type 'bool'` * The `if (_swingh != _swingh_prev)` "only send it if it changed" check never settles, so Swing(H) is either re-sent on every single send, or never sent at all -- decided by whatever was on the stack that day. This is also why `TestIRac.LG2_AKB73757604` is flaky: it asserts six messages get sent, and the sixth is exactly that Swing(H) message. It passes or fails depending on that garbage value, which is a fun way to spend an afternoon when it fails on an unrelated PR. :-) Fixed by copying `_swingh` across in `updateSwingPrev()` like its siblings. Added `TestIRLgAcClass.SwingHPrevIsTracked`, which fails on master regardless of what the uninitialised value happens to be, since it checks a repeated send doesn't re-send Swing(H). * LG: Trim the new test's comments down to one-liners The rationale belongs in the commit message & PR, not wrapped across four lines above the test. Matches the rest of the file, where tests carry either no comment or a single line.
1 parent 222afce commit 1aa31ea

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

src/ir_LG.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ void IRLgAc::setSwingV(const uint32_t position) {
591591
// Copy the previous swing settings from the current ones.
592592
void IRLgAc::updateSwingPrev(void) {
593593
_swingv_prev = _swingv;
594+
_swingh_prev = _swingh;
594595
for (uint8_t i = 0; i < kLgAcSwingVMaxVanes; i++)
595596
_vaneswingv_prev[i] = _vaneswingv[i];
596597
}

test/ir_LG_test.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,3 +1190,31 @@ TEST(TestIRLgAcClass, SwingVToggle) {
11901190
EXPECT_EQ(ac._swingv, kLgAcSwingVToggle);
11911191
EXPECT_NE(ac._swingv_prev, kLgAcSwingVToggle);
11921192
}
1193+
1194+
// Ensure Swing(H)'s previous state is initialised & kept up to date.
1195+
TEST(TestIRLgAcClass, SwingHPrevIsTracked) {
1196+
IRLgAc ac(kGpioUnused);
1197+
ac.begin();
1198+
ac.setModel(lg_ac_remote_model_t::AKB73757604);
1199+
ac.setPower(true);
1200+
ac.setMode(kLgAcCool);
1201+
ac.setTemp(25);
1202+
ac.setFan(kLgAcFanLow);
1203+
ac.setSwingH(true);
1204+
1205+
ac._irsend.reset();
1206+
ac.send();
1207+
ac._irsend.makeDecodeResult();
1208+
EXPECT_EQ(121, ac._irsend.capture.rawlen); // Normal + Swing(H) message.
1209+
1210+
ac._irsend.reset();
1211+
ac.send();
1212+
ac._irsend.makeDecodeResult();
1213+
EXPECT_EQ(61, ac._irsend.capture.rawlen); // Unchanged, so normal only.
1214+
1215+
ac.setSwingH(false);
1216+
ac._irsend.reset();
1217+
ac.send();
1218+
ac._irsend.makeDecodeResult();
1219+
EXPECT_EQ(121, ac._irsend.capture.rawlen); // Changed again, so both.
1220+
}

0 commit comments

Comments
 (0)