Skip to content

refactor: decouple state setter from async callback execution#506

Merged
firstof9 merged 5 commits intomainfrom
websocket-optimization
Feb 2, 2026
Merged

refactor: decouple state setter from async callback execution#506
firstof9 merged 5 commits intomainfrom
websocket-optimization

Conversation

@firstof9
Copy link
Owner

@firstof9 firstof9 commented Feb 2, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved websocket connection stability and state transition reliability, adding a thread-safe fallback for environments without a running event loop and ensuring transient error reasons are cleared on state change.
  • Tests

    • Updated and expanded test coverage for websocket state management, including a new test validating the thread-safe fallback behavior and related state resets.

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

Refactors WebSocket state management: replaces an async state setter with a synchronous setter that schedules async callbacks via asyncio.create_task and falls back to loop.call_soon_threadsafe. Adds an internal async _set_state helper and tests for the threadsafe fallback and import cleanup.

Changes

Cohort / File(s) Summary
WebSocket State Management Refactor
openevsehttp/websocket.py
Converted async state setter to a synchronous setter that schedules the callback via asyncio.create_task with a fallback to loop.call_soon_threadsafe. Added async def _set_state(self, value) and migrated state transitions in running(), close(), keepalive(), and error paths to use the helper. Reset _error_reason consistently after scheduling.
Test Updates
tests/test_websocket.py
Added test_state_setter_threadsafe_fallback to simulate asyncio.create_task raising RuntimeError and verify the fallback uses a mocked event loop's call_soon_threadsafe, and that _error_reason is cleared.
Test Cleanup
tests/conftest.py
Removed an unused json import.
Test Rename
tests/test_main.py
Renamed test_get_status to test_get_status_error.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 State hops from sync to async with cheer,
I queue the task when no loop is near.
If create_task trips and cannot run,
call_soon_threadsafe saves the fun.
Error reasons cleared — the run stays clear. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring work: decoupling the state setter from async callback execution, which is the primary change across multiple files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch websocket-optimization

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Pylint (4.0.4)
tests/test_websocket.py

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@firstof9 firstof9 added the code quality Improves code quality label Feb 2, 2026
@firstof9 firstof9 marked this pull request as ready for review February 2, 2026 14:01
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tests/test_websocket.py`:
- Around line 285-306: The test function test_state_setter_threadsafe_fallback
has an unused fixture parameter mock_callback; remove mock_callback from the
test signature so it becomes async def
test_state_setter_threadsafe_fallback(ws_client): to satisfy Ruff ARG001,
leaving ws_client (which already depends on the callback fixture) intact; update
any test references if necessary and run the linter to confirm the warning is
resolved.

@firstof9 firstof9 changed the title refactor: optimize websocket handling refactor: decouple state setter from async callback execution Feb 2, 2026
Repository owner deleted a comment from coderabbitai bot Feb 2, 2026
@firstof9
Copy link
Owner Author

firstof9 commented Feb 2, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@firstof9 firstof9 merged commit a16fad4 into main Feb 2, 2026
5 checks passed
@firstof9 firstof9 deleted the websocket-optimization branch February 2, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality Improves code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant