8000
Skip to content

Remove dead code: 64-bit protobuf types never used in 7 years - #9471

Merged
bdraco merged 1 commit into
devfrom
wire_type_1
Jul 16, 2025
Merged

Remove dead code: 64-bit protobuf types never used in 7 years#9471
bdraco merged 1 commit into
devfrom
wire_type_1

Conversation

@bdraco
@bdraco bdraco commented Jul 13, 2025
Copy link
Copy Markdown
Member

What does this implement/fix?

Remove dead code for protobuf wire type 1 (64-bit fixed types) that has never been used in ESPHome's ~7 year history.

The ESPHome API has never used double, fixed64, sfixed64, or sint64 types in its protocol definition. This PR removes all the encoding/decoding infrastructure for these types and adds validation to prevent their accidental introduction.

Key points:

  • These types have been dead code since ESPHome's inception (~7 years)
  • ESP32/ESP8266 are 32-bit microcontrollers - 64-bit operations are expensive
  • Saves 112 bytes of flash memory (small but every byte counts on embedded systems)
  • Prevents future use without careful consideration
  • Support can be re-added if truly needed (unlikely as ESP32 isn't becoming 64-bit)

Changes:

  • Removed Proto64Bit class entirely
  • Removed encode_fixed64() method
  • Removed add_sint64_field() and add_sint64_field_repeated() functions
  • Removed decode_64bit() virtual method
  • Added validation in api_protobuf.py that raises ValueError if these types are used
  • Added comments documenting the intentional omission

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable): N/A

Pull request in esphome-docs with documentation (if applicable): N/A

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

# No configuration changes needed
api:

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

Uh oh!

There was an error while loading. Please reload this page.

Copilot AI review requested due to automatic review settings July 13, 2025 20:19
@bdraco
bdraco requested a review from OttoWinter as a code owner July 13, 2025 20:19
@probot-esphome
Copy link
Copy Markdown

Hey there @OttoWinter, mind taking a look at this pull request as it has been labeled with an integration (api) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Remove dead protobuf support for 64-bit types and prevent accidental reintroduction by adding validation in the Python API generator and cleaning up the C++ code.

  • Eliminate Proto64Bit, 64-bit encode/decode methods, and size calculations for sint64/fixed64 in proto.h.
  • Introduce validate_field_type in api_protobuf.py to raise on unsupported 64-bit field types and refactor type info lookups.
  • Update comments to document the intentional omission of wire type 1 support.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
script/api_protobuf/api_protobuf.py Added UNSUPPORTED_TYPES map, validate_field_type, and refactored repeated/type lookup to enforce no 64-bit fields.
esphome/components/api/proto.h Removed Proto64Bit, 64-bit encode/decode methods, and sint64 size functions, with comments noting the removal.
Comments suppressed due to low confidence (2)

script/api_protobuf/api_protobuf.py:321

  • No tests exist to verify that validate_field_type raises ValueError for unsupported 64-bit types; consider adding unit tests to cover each unsupported type code to prevent regressions.
def validate_field_type(field_type: int, field_name: str = "") -> None:

script/api_protobuf/api_protobuf.py:342

  • Avoid using magic numbers for field labels; use the constant descriptor.FieldDescriptorProto.LABEL_REPEATED instead of literal 3 to improve readability and maintainability.
    if field.label == 3:  # repeated

@codecov-commenter
codecov-commenter commented Jul 13, 2025
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.67%. Comparing base (84956b6) to head (75ef572).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #9471   +/-   ##
=======================================
  Coverage   59.67%   59.67%           
=======================================
  Files          50       50           
  Lines       10261    10261           
  Branches     1379     1379           
=======================================
  Hits         6123     6123           
  Misses       3778     3778           
  Partials      360      360           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bdraco
bdraco merged commit 856cb18 into dev Jul 16, 2025
37 checks passed
@bdraco
bdraco deleted the wire_type_1 branch July 16, 2025 01:12
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

0