8000
Skip to content

Improve API protobuf decode method readability and reduce code size - #9455

Merged
jesserockz merged 2 commits into
devfrom
reformat_api_jump_tables
Jul 16, 2025
Merged

Improve API protobuf decode method readability and reduce code size#9455
jesserockz merged 2 commits into
devfrom
reformat_api_jump_tables

Conversation

@bdraco
@bdraco bdraco commented Jul 12, 2025 •
edited
Loading
Copy link
Copy Markdown
Member

What does this implement/fix?

This PR refactors the code generation for API protocol buffer decode methods to produce more readable and compact code. The switch-case statements are now formatted with single-line case statements instead of multi-line blocks, making the code easier to read while maintaining the same functionality.

The changes reduce the generated api_pb2.cpp file by approximately 6KB (457 lines), making it both more readable and more compact. (a lot less now after #9461)

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 (internal code generation change only)

Test Environment

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

Example entry for config.yaml:

# No configuration changes required - this is an internal code generation improvement

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:

  • N/A - This is an internal code generation change with no user-facing impact
8000

Additional Notes

Before:

bool UpdateStateResponse::decode_varint(uint32_t field_id, ProtoVarInt value) {
  switch (field_id) {
    case 2: {
      this->missing_state = value.as_bool();
      return true;
    }
    case 3: {
      this->in_progress = value.as_bool();
      return true;
    }
    // ... more cases
    default:
      return false;
  }
}

After:

bool UpdateStateResponse::decode_varint(uint32_t field_id, ProtoVarInt value) {
  switch (field_id) {
    case 2:
      this->missing_state = value.as_bool();
      break;
    case 3:
      this->in_progress = value.as_bool();
      break;
    case 4:
      this->has_progress = value.as_bool();
      break;
    case 11:
      this->device_id = value.as_uint32();
      break;
    default:
      return false;
  }
  return true;
}

The compiler generates the same efficient jump table in both cases, but the new format is significantly more readable and reduces file size.

@codecov-commenter
codecov-commenter commented Jul 12, 2025 •
edited
Loading
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.63%. Comparing base (f4ac951) to head (4c0c095).

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev    #9455      +/-   ##
==========================================
+ Coverage   59.60%   59.63%   +0.02%     
==========================================
  Files          50       50              
  Lines       10261    10261              
  Branches     1379     1379              
==========================================
+ Hits         6116     6119       +3     
+ Misses       3786     3784       -2     
+ Partials      359      358       -1

☔ 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 marked this pull request as ready for review July 12, 2025 05:17
Copilot AI review requested due to automatic review settings July 12, 2025 05:17
@bdraco
bdraco requested a review from OttoWinter as a code owner July 12, 2025 05:17
@probot-esphome
probot-esphome Bot commented Jul 12, 2025
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 reviewed Jul 12, 2025
View reviewed changes
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

This PR refactors the Python code generator for API protocol buffer decode methods to emit single-line case statements, remove the dedent dependency, and move default/return logic outside of switch blocks for more compact and readable output.

  • Removed textwrap.dedent and replaced multi-line case templates with one-line templates ending in break;
  • Streamlined build_message_type to append default and final return outside the switch
  • Reduced generated C++ file size by ~6 KB (457 lines)
Comments suppressed due to low confidence (1)

script/api_protobuf/api_protobuf.py:155

  • [nitpick] These decode_*_content methods all follow the same pattern of emitting case {number}: ...; break;. Consider extracting a helper function (e.g. def _single_line_case(number, field_name, expr):) to generate this string and avoid duplicating the template logic.
    def decode_varint_content(self) -> str:

Comment thread script/api_protobuf/api_protobuf.py
@bdraco
bdraco marked this pull request as draft July 12, 2025 20:36
@bdraco
bdraco commented Jul 12, 2025
Copy link
Copy Markdown
Member Author

waits for #9461

@bdraco
bdraco marked this pull request as ready for review July 12, 2025 21:05
@jesserockz
jesserockz merged commit b5be452 into dev Jul 16, 2025
31 checks passed
@jesserockz
jesserockz deleted the reformat_api_jump_tables branch July 16, 2025 01:15
@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.

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@jesserockz jesserockz jesserockz approved these changes
@OttoWinter OttoWinter Awaiting requested review from OttoWinter

Assignees

@OttoWinter OttoWinter

Labels

component: api core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bdraco @codecov-commenter @jesserockz @OttoWinter
0