diff --git a/esphome/components/remote_base/__init__.py b/esphome/components/remote_base/__init__.py index 1a767b723d7..5b6cb33dec4 100644 --- a/esphome/components/remote_base/__init__.py +++ b/esphome/components/remote_base/__init__.py @@ -953,16 +953,22 @@ MAGIQUEST_SCHEMA = cv.Schema( } ) +# A wand_id of 0 matches any wand. Magnitude is still accepted but not matched on. +MAGIQUEST_BINARY_SENSOR_SCHEMA = cv.Schema( + { + cv.Optional(CONF_WAND_ID, default=0): cv.hex_uint32_t, + cv.Optional(CONF_MAGNITUDE): cv.hex_uint16_t, + } +) -@register_binary_sensor("magiquest", MagiQuestBinarySensor, MAGIQUEST_SCHEMA) + +@register_binary_sensor( + "magiquest", MagiQuestBinarySensor, MAGIQUEST_BINARY_SENSOR_SCHEMA +) def magiquest_binary_sensor(var, config): cg.add( var.set_data( - cg.StructInitializer( - MagiQuestData, - ("magnitude", config[CONF_MAGNITUDE]), - ("wand_id", config[CONF_WAND_ID]), - ) + cg.StructInitializer(MagiQuestData, ("wand_id", config[CONF_WAND_ID])) ) ) diff --git a/esphome/components/remote_base/magiquest_protocol.cpp b/esphome/components/remote_base/magiquest_protocol.cpp index 5b47f7e438f..71a6a5ace1b 100644 --- a/esphome/components/remote_base/magiquest_protocol.cpp +++ b/esphome/components/remote_base/magiquest_protocol.cpp @@ -2,7 +2,7 @@ #include "esphome/core/log.h" /* Based on protocol analysis from - * https://arduino-irremote.github.io/Arduino-IRremote/ir__MagiQuest_8cpp_source.html + * https://arduino-irremote.github.io/Arduino-IRremote/ir__MagiQuest_8hpp_source.html */ namespace esphome::remote_base { @@ -10,12 +10,25 @@ namespace esphome::remote_base { ESPHOME_LOG_TAG(TAG, "remote.magiquest"); static constexpr uint32_t MAGIQUEST_UNIT = 288; // us +// Half a unit is the widest window that still tells a one unit mark or space from a two unit one. +static constexpr uint32_t MAGIQUEST_TOLERANCE = MAGIQUEST_UNIT / 2; static constexpr uint32_t MAGIQUEST_ONE_MARK = 2 * MAGIQUEST_UNIT; static constexpr uint32_t MAGIQUEST_ONE_SPACE = 2 * MAGIQUEST_UNIT; static constexpr uint32_t MAGIQUEST_ZERO_MARK = MAGIQUEST_UNIT; static constexpr uint32_t MAGIQUEST_ZERO_SPACE = 3 * MAGIQUEST_UNIT; +static bool checksum_is_valid(uint32_t wand_id, uint32_t magnitude_and_checksum) { + // The frame's six payload bytes, which start at the wand id's top bit, must sum to zero. + const uint32_t id_bytes = wand_id << 1; + uint8_t sum = 0; + for (uint8_t shift = 0; shift < 32; shift += 8) { + sum += (id_bytes >> shift) + (magnitude_and_checksum >> shift); + } + return sum == 0; +} + void MagiQuestProtocol::encode(RemoteTransmitData *dst, const MagiQuestData &data) { + // Still the old frame layout, which the decoder no longer reads; changing it would alter what existing configs send. dst->reserve(101); // 2 start bits, 48 data bits, 1 stop bit dst->set_carrier_frequency(38000); @@ -41,41 +54,48 @@ void MagiQuestProtocol::encode(RemoteTransmitData *dst, const MagiQuestData &dat dst->mark(MAGIQUEST_UNIT); } optional MagiQuestProtocol::decode(RemoteReceiveData src) { - MagiQuestData data{ - .magnitude = 0, - .wand_id = 0, - }; - // Two start bits - if (!src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE) || - !src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { - return {}; + src.set_tolerance(MAGIQUEST_TOLERANCE, TOLERANCE_MODE_TIME); + + MagiQuestData data{}; + + // 8 bit header, always zero + for (uint8_t i = 0; i < 8; i++) { + if (!src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { + return {}; + } } - for (uint32_t mask = 1 << 31; mask; mask >>= 1) { + // 31 bit wand id + for (uint32_t mask = 1 << 30; mask; mask >>= 1) { if (src.expect_item(MAGIQUEST_ONE_MARK, MAGIQUEST_ONE_SPACE)) { data.wand_id |= mask; - } else if (src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { - data.wand_id &= ~mask; - } else { + } else if (!src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { return {}; } } - for (uint16_t mask = 1 << 15; mask; mask >>= 1) { + // 9 bit magnitude, then 8 bit checksum + uint32_t magnitude_and_checksum = 0; + for (uint32_t mask = 1 << 16; mask > 1; mask >>= 1) { if (src.expect_item(MAGIQUEST_ONE_MARK, MAGIQUEST_ONE_SPACE)) { - data.magnitude |= mask; - } else if (src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { - data.magnitude &= ~mask; - } else { + magnitude_and_checksum |= mask; + } else if (!src.expect_item(MAGIQUEST_ZERO_MARK, MAGIQUEST_ZERO_SPACE)) { return {}; } } + // The last bit has no trailing space. A wrong or missing mark here is caught by the checksum. + if (src.expect_mark(MAGIQUEST_ONE_MARK)) { + magnitude_and_checksum |= 1; + } - src.expect_mark(MAGIQUEST_UNIT); + if (!checksum_is_valid(data.wand_id, magnitude_and_checksum)) { + return {}; + } + data.magnitude = magnitude_and_checksum >> 8; return data; } void MagiQuestProtocol::dump(const MagiQuestData &data) { - ESP_LOGI(TAG, "Received MagiQuest: wand_id=0x%08" PRIX32 ", magnitude=0x%04X", data.wand_id, data.magnitude); + ESP_LOGI(TAG, "Received MagiQuest: wand_id=0x%08" PRIX32 ", magnitude=%u", data.wand_id, data.magnitude); } } // namespace esphome::remote_base diff --git a/esphome/components/remote_base/magiquest_protocol.h b/esphome/components/remote_base/magiquest_protocol.h index 98a5fe20819..4e88f06cd45 100644 --- a/esphome/components/remote_base/magiquest_protocol.h +++ b/esphome/components/remote_base/magiquest_protocol.h @@ -5,23 +5,24 @@ #include /* Based on protocol analysis from - * https://arduino-irremote.github.io/Arduino-IRremote/ir__MagiQuest_8cpp_source.html + * https://arduino-irremote.github.io/Arduino-IRremote/ir__MagiQuest_8hpp_source.html */ namespace esphome::remote_base { +// Older decoders read the wand id 5 bits too early, so ids in older configs lack its 5 low bits. +static constexpr uint8_t MAGIQUEST_LEGACY_WAND_ID_SHIFT = 5; + struct MagiQuestData { uint16_t magnitude; uint32_t wand_id; + // Not symmetric: `this` is the decoded frame and `rhs` the configured match. A configured wand_id of 0 matches + // any wand. Magnitude is not compared; it appears to encode the cast type rather than a strength. The legacy + // match applies to every configured id, so an id below 2^26 also matches the 32 wands whose full id shifts to it. bool operator==(const MagiQuestData &rhs) const { - // Treat 0xffff as a special, wildcard magnitude - // In testing, the wand never produces this value, and this allows us to match - // on just the wand_id if wanted. - if (rhs.wand_id != this->wand_id) { - return false; - } - return (this->wand_id == 0xffff || rhs.wand_id == 0xffff || this->wand_id == rhs.wand_id); + return rhs.wand_id == 0 || rhs.wand_id == this->wand_id || + rhs.wand_id == (this->wand_id >> MAGIQUEST_LEGACY_WAND_ID_SHIFT); } }; diff --git a/tests/components/remote_base/__init__.py b/tests/components/remote_base/__init__.py index 31ecbc3eac0..82840b5c2b4 100644 --- a/tests/components/remote_base/__init__.py +++ b/tests/components/remote_base/__init__.py @@ -14,6 +14,7 @@ def override_manifest(manifest: ComponentManifestOverride) -> None: "aeha", "keeloq", "nexus", + "magiquest", "rc5x", "panasonic", ): diff --git a/tests/components/remote_base/magiquest_test.cpp b/tests/components/remote_base/magiquest_test.cpp new file mode 100644 index 00000000000..3641f1ec807 --- /dev/null +++ b/tests/components/remote_base/magiquest_test.cpp @@ -0,0 +1,126 @@ +#include +#include "esphome/components/remote_base/magiquest_protocol.h" + +namespace esphome::remote_base::testing { + +namespace { + +constexpr int32_t UNIT = 288; +constexpr uint32_t WAND_ID = 0x2A5B3C1D; +constexpr uint16_t MAGNITUDE = 258; +constexpr MagiQuestData DECODED{.magnitude = MAGNITUDE, .wand_id = WAND_ID}; + +void add_bit(RawTimings &timings, bool one, bool last = false) { + timings.push_back(one ? 2 * UNIT : UNIT); + if (!last) { + timings.push_back(-(one ? 2 * UNIT : 3 * UNIT)); + } +} + +// Overwrite bit `bit` of a built frame with a one +void set_one(RawTimings &timings, size_t bit) { + timings[2 * bit] = 2 * UNIT; + timings[2 * bit + 1] = -2 * UNIT; +} + +// Build a frame the way a wand sends it: 8 zero header bits, a 31 bit wand id, a 9 bit magnitude and a checksum +// that makes the six payload bytes sum to zero. The last bit has no trailing space. +RawTimings build_frame(uint32_t wand_id, uint16_t magnitude, uint8_t checksum_offset = 0) { + const uint32_t id_bytes = wand_id << 1; + const uint32_t magnitude_bits = uint32_t(magnitude) << 8; + uint8_t sum = 0; + for (uint8_t shift = 0; shift < 32; shift += 8) { + sum += (id_bytes >> shift) + (magnitude_bits >> shift); + } + const uint32_t magnitude_and_checksum = magnitude_bits | uint8_t(-sum + checksum_offset); + + RawTimings timings; + for (int i = 0; i < 8; i++) { + add_bit(timings, false); + } + for (uint32_t mask = 1 << 30; mask; mask >>= 1) { + add_bit(timings, wand_id & mask); + } + for (uint32_t mask = 1 << 16; mask; mask >>= 1) { + add_bit(timings, magnitude_and_checksum & mask, mask == 1); + } + return timings; +} + +// The receiver's tolerance is passed for completeness; decode() replaces it with the protocol's fixed window +optional decode(const RawTimings &timings) { + return MagiQuestProtocol().decode(RemoteReceiveData(timings, 25, TOLERANCE_MODE_PERCENTAGE)); +} + +void expect_decodes_to(const RawTimings &timings, uint32_t wand_id, uint16_t magnitude) { + auto decoded = decode(timings); + ASSERT_TRUE(decoded.has_value()); + // clang-tidy's unchecked-optional-access models neither gtest's ASSERT_TRUE nor value() as a check + if (decoded.has_value()) { + EXPECT_EQ(decoded->wand_id, wand_id); + EXPECT_EQ(decoded->magnitude, magnitude); + } +} + +} // namespace + +TEST(MagiQuestProtocolTest, DecodesAValidFrame) { + expect_decodes_to(build_frame(WAND_ID, MAGNITUDE), WAND_ID, MAGNITUDE); +} + +// Magnitude 1 with wand id 0 needs a checksum of 0xFF, so the last bit is a one; magnitude 2 needs 0xFE. +TEST(MagiQuestProtocolTest, DecodesBothValuesOfTheLastBit) { + expect_decodes_to(build_frame(0, 1), 0, 1); + expect_decodes_to(build_frame(0, 2), 0, 2); +} + +TEST(MagiQuestProtocolTest, RejectsABadChecksum) { + EXPECT_FALSE(decode(build_frame(WAND_ID, MAGNITUDE, 1)).has_value()); +} + +TEST(MagiQuestProtocolTest, RejectsAFlippedDataBit) { + auto timings = build_frame(WAND_ID, MAGNITUDE); + set_one(timings, 8); // first wand id bit, a zero in WAND_ID + EXPECT_FALSE(decode(timings).has_value()); +} + +TEST(MagiQuestProtocolTest, RejectsANonZeroHeader) { + auto timings = build_frame(WAND_ID, MAGNITUDE); + set_one(timings, 0); + EXPECT_FALSE(decode(timings).has_value()); +} + +TEST(MagiQuestProtocolTest, RejectsATruncatedFrame) { + auto timings = build_frame(WAND_ID, MAGNITUDE); + timings.resize(timings.size() - 10); + EXPECT_FALSE(decode(timings).has_value()); +} + +// Receivers stretch marks and shrink spaces; 120 us is more than 25% of a unit but within the fixed window. +TEST(MagiQuestProtocolTest, DecodesWithReceiverJitterUnderTheDefaultTolerance) { + auto timings = build_frame(WAND_ID, MAGNITUDE); + for (auto &timing : timings) { + timing += 120; + } + expect_decodes_to(timings, WAND_ID, MAGNITUDE); +} + +TEST(MagiQuestProtocolTest, MatchesAConfiguredWand) { + EXPECT_TRUE(DECODED == (MagiQuestData{.magnitude = 0, .wand_id = WAND_ID})); + EXPECT_FALSE(DECODED == (MagiQuestData{.magnitude = 0, .wand_id = WAND_ID + 1})); +} + +TEST(MagiQuestProtocolTest, MatchesAnyWandWhenNoIdIsConfigured) { + EXPECT_TRUE(DECODED == (MagiQuestData{.magnitude = 0, .wand_id = 0})); +} + +// Configs written for the old decoder hold the id without its 5 low bits. +TEST(MagiQuestProtocolTest, MatchesAnIdFromTheOldDecoder) { + EXPECT_TRUE(DECODED == (MagiQuestData{.magnitude = 0, .wand_id = WAND_ID >> 5})); +} + +TEST(MagiQuestProtocolTest, IgnoresMagnitudeWhenMatching) { + EXPECT_TRUE(DECODED == (MagiQuestData{.magnitude = 0xFFFF, .wand_id = WAND_ID})); +} + +} // namespace esphome::remote_base::testing diff --git a/tests/components/remote_receiver/esp32-common.yaml b/tests/components/remote_receiver/esp32-common.yaml index da9a0fc9630..f66c2ed3b0b 100644 --- a/tests/components/remote_receiver/esp32-common.yaml +++ b/tests/components/remote_receiver/esp32-common.yaml @@ -32,6 +32,13 @@ binary_sensor: name: Hob2Hood Light On hob2hood: command: light_on + - platform: remote_receiver + name: MagiQuest Wand + magiquest: + wand_id: 0x0F1E2D3C + - platform: remote_receiver + name: MagiQuest Any Wand + magiquest: {} - platform: remote_receiver name: Nexus Sensor nexus: