Address review feedback: fix unconditional guard bug, add ifndef guard, clarify comment

- Fix get_varint64_ifdef() to return unconditional guard when any 64-bit
  varint field has no ifdef (None in ifdefs set)
- Wrap USE_API_VARINT64 define with #ifndef to avoid redefinition warnings
  when esphome/core/defines.h also defines it (test/IDE builds)
- Clarify proto.h comment about uint32_t shift behavior at byte 4
- Add namespace to generated api_pb2_defines.h for linter compliance
This commit is contained in:
J. Nick Koston
2026-02-19 20:23:17 -06:00
parent 4e3b2abd5f
commit 21b9a4139a
3 changed files with 15 additions and 4 deletions
+4
View File
@@ -4,5 +4,9 @@
#include "esphome/core/defines.h"
#ifdef USE_BLUETOOTH_PROXY
#ifndef USE_API_VARINT64
#define USE_API_VARINT64
#endif
#endif
namespace esphome::api {} // namespace esphome::api
+1 -1
View File
@@ -119,7 +119,7 @@ class ProtoVarInt {
}
// 32-bit phase: process remaining bytes with native 32-bit shifts.
// Without USE_API_VARINT64: cover bytes 1-4 (shifts 7, 14, 21, 28) — the uint32_t
// shift at byte 4 truncates upper bits but those are always zero for valid uint32 values.
// shift at byte 4 (shift by 28) may lose bits 32-34, but those are always zero for valid uint32 values.
// With USE_API_VARINT64: cover bytes 1-3 (shifts 7, 14, 21) so parse_wide handles
// byte 4+ with full 64-bit arithmetic (avoids truncating values > UINT32_MAX).
uint32_t result32 = buffer[0] & 0x7F;
+10 -3
View File
@@ -1929,6 +1929,9 @@ def get_varint64_ifdef(
}
if not ifdefs:
return False, None
if None in ifdefs:
# At least one 64-bit varint field is unconditional, so the guard must be unconditional.
return True, None
ifdefs.discard(None)
return True, ifdefs.pop() if len(ifdefs) == 1 else None
@@ -2603,10 +2606,14 @@ def main() -> None:
defines_content += "#pragma once\n\n"
defines_content += '#include "esphome/core/defines.h"\n'
if has_varint64:
defines_content += "\n".join(
wrap_with_ifdef(["#define USE_API_VARINT64"], varint64_guard)
)
lines = [
"#ifndef USE_API_VARINT64",
"#define USE_API_VARINT64",
"#endif",
]
defines_content += "\n".join(wrap_with_ifdef(lines, varint64_guard))
defines_content += "\n"
defines_content += "\nnamespace esphome::api {} // namespace esphome::api\n"
with open(root / "api_pb2_defines.h", "w", encoding="utf-8") as f:
f.write(defines_content)