Skip to content

Conversation

@jesserockz
Copy link
Member

@jesserockz jesserockz commented Apr 15, 2025

What does this implement/fix?

Draft to reserve proto IDs

This change allows a plaintext API client to generate and send a desired encryption key that the device should use from now. All clients will be gracefully disconnected once the device saves the key and will need to reconnect with the new key.

Related PRs:

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):

  • fixes

Pull request in esphome (if applicable):

  • esphome/esphome#

Checklist:

  • The code change is tested and works locally.
  • If api.proto was modified, a linked pull request has been made to esphome with the same changes.
  • Tests have been added to verify that the new code works (under tests/ folder).

@codecov
Copy link

codecov bot commented Apr 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (30b7710) to head (cd399b3).
Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1151   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           19        19           
  Lines         2783      2789    +6     
=========================================
+ Hits          2783      2789    +6     

☔ 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.

@codspeed-hq
Copy link

codspeed-hq bot commented Apr 15, 2025

CodSpeed Performance Report

Merging #1151 will not alter performance

Comparing jesserockz-2025-053 (cd399b3) with main (9b44b52)

Summary

✅ 9 untouched benchmarks

@bdraco
Copy link
Member

bdraco commented Apr 20, 2025

Can we merge this now?

@jesserockz
Copy link
Member Author

I am working on this today to get the actual python code implemented

@jesserockz jesserockz marked this pull request as ready for review April 28, 2025 01:17
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 28, 2025

Walkthrough

The changes introduce support for a new noise encryption key management feature in the API. This involves extending the protobuf API definition to add a new RPC method for setting a noise encryption key, along with corresponding request and response message types. The client library is updated to provide an asynchronous method for invoking this RPC. The protocol core is updated to recognize the new message types. The API model layer adds dataclasses for the new request and response types. Tests are added and updated to cover the new client method and model-to-protobuf conversions.

Changes

File(s) Change Summary
aioesphomeapi/api.proto Added new RPC noise_encryption_set_key to APIConnection service; introduced NoiseEncryptionSetKeyRequest and NoiseEncryptionSetKeyResponse messages; added api_encryption_supported field to DeviceInfoResponse.
aioesphomeapi/client.py Added noise_encryption_set_key async method to APIClient for sending a noise encryption key to the device and handling the response.
aioesphomeapi/core.py Registered NoiseEncryptionSetKeyRequest and NoiseEncryptionSetKeyResponse in protocol message mappings.
aioesphomeapi/model.py Added dataclasses NoiseEncryptionSetKeyRequest and NoiseEncryptionSetKeyResponse for the new API feature.
tests/test_client.py Added async test test_noise_encryption_set_key to verify client behavior for the new RPC; imported necessary protobuf message classes.
tests/test_model.py Added test coverage for NoiseEncryptionSetKeyResponse model and its protobuf conversion in parameterized tests; updated imports.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant APIClient
    participant Device

    Client->>APIClient: noise_encryption_set_key(key)
    APIClient->>Device: NoiseEncryptionSetKeyRequest(key)
    Device-->>APIClient: NoiseEncryptionSetKeyResponse(success)
    APIClient-->>Client: Return success
Loading

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac1f1d7 and cd399b3.

📒 Files selected for processing (6)
  • aioesphomeapi/api.proto (3 hunks)
  • aioesphomeapi/client.py (3 hunks)
  • aioesphomeapi/core.py (2 hunks)
  • aioesphomeapi/model.py (1 hunks)
  • tests/test_client.py (2 hunks)
  • tests/test_model.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**`: - Do not generate or add any sequence diagrams

**: - Do not generate or add any sequence diagrams

  • aioesphomeapi/core.py
  • tests/test_model.py
  • aioesphomeapi/client.py
  • aioesphomeapi/model.py
  • tests/test_client.py
  • aioesphomeapi/api.proto
🧬 Code Graph Analysis (3)
aioesphomeapi/core.py (1)
aioesphomeapi/model.py (2)
  • NoiseEncryptionSetKeyRequest (1339-1340)
  • NoiseEncryptionSetKeyResponse (1344-1345)
tests/test_model.py (1)
aioesphomeapi/model.py (1)
  • NoiseEncryptionSetKeyResponse (1344-1345)
aioesphomeapi/client.py (3)
aioesphomeapi/model.py (4)
  • NoiseEncryptionSetKeyRequest (1339-1340)
  • NoiseEncryptionSetKeyResponse (1344-1345)
  • from_pb (91-92)
  • from_pb (1089-1132)
aioesphomeapi/client_base.py (1)
  • _get_connection (317-325)
aioesphomeapi/connection.py (1)
  • send_message_await_response (840-850)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: py 3.12 on windows-latest (skip_cython)
🔇 Additional comments (14)
aioesphomeapi/api.proto (3)

34-34: LGTM: New RPC method for noise encryption key management

This addition aligns well with the existing API structure, providing a dedicated method for setting the noise encryption key.


232-233: LGTM: Good addition of device capability field

Adding the api_encryption_supported boolean field to DeviceInfoResponse is an appropriate way to indicate capability support in the device discovery phase.


645-660: LGTM: Well-designed message types for noise encryption

The new message types follow the established pattern:

  • NoiseEncryptionSetKeyRequest with a bytes field for the key
  • NoiseEncryptionSetKeyResponse with a boolean field for success indication
  • Both messages are properly tagged with feature flag USE_API_NOISE
  • Appropriate IDs assigned (124 and 125)
aioesphomeapi/core.py (2)

92-93: LGTM: Proper import of new message types

The imports are correctly placed in alphabetical order within the import block.


443-444: LGTM: Correct mapping of message types to IDs

The message types are properly mapped to their IDs (124 and 125) in the MESSAGE_TYPE_TO_PROTO dictionary, which will enable the protocol to correctly handle these messages.

tests/test_model.py (3)

49-49: LGTM: Added import for new protobuf message type

The import is correctly placed in alphabetical order within the protobuf imports block.


100-100: LGTM: Added import for new model class

The model class import is correctly placed in alphabetical order within the model imports block.


296-296: LGTM: Added test case for model-protobuf conversion

Including the new model in the parameterized test ensures that conversion between the model and protobuf message is properly tested.

aioesphomeapi/model.py (1)

1338-1346: LGTM: Well-structured model classes for noise encryption

The new model classes:

  • Follow the established pattern of frozen dataclasses
  • Include appropriate field types (bytes for key, bool for success)
  • Set appropriate default values (empty bytes for key, False for success)
  • Include proper pylint disable comment for the bytes field
aioesphomeapi/client.py (3)

55-56: LGTM! Added new protobuf message types for noise encryption key functionality.

The additions properly import the required message types from the protobuf definitions.


135-135: LGTM! Added model import for noise encryption response.

The import follows the established pattern of importing protobuf-to-model conversion classes with a clear, descriptive alias.


1380-1389: LGTM! Well-implemented noise encryption key setter method.

The method follows the established patterns in the codebase:

  1. Creates a request object with the provided key
  2. Awaits a response from the device
  3. Converts the protobuf response to a model object
  4. Returns the success status

The implementation is clean, concise, and handles errors consistently with other methods through _get_connection().

tests/test_client.py (2)

59-60: LGTM! Added protobuf message types for testing.

The imports mirror those in the client implementation file, maintaining consistency.


2740-2759: LGTM! Comprehensive test for noise encryption key setting.

The test follows the established testing pattern for client methods:

  1. Patches the connection's send_message method to verify the correct request is sent
  2. Creates a task that calls the new API method
  3. Simulates receiving a successful response from the device
  4. Verifies the method returns the expected result (True)

This provides good coverage for the new functionality.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@jesserockz jesserockz merged commit 463b136 into main Apr 28, 2025
19 checks passed
@jesserockz jesserockz deleted the jesserockz-2025-053 branch April 28, 2025 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants