Skip to content

Split WaterInfoEvent into multiple#917

Merged
edenhaus merged 2 commits into
devfrom
split-WaterInfoEvent
Apr 25, 2025
Merged

Split WaterInfoEvent into multiple#917
edenhaus merged 2 commits into
devfrom
split-WaterInfoEvent

Conversation

@edenhaus

Copy link
Copy Markdown
Member

A different approach for #905

@nanomad What do you think about this approach?

@edenhaus edenhaus added the pr: Breaking Change Pull request with braking changes label Apr 22, 2025
@edenhaus edenhaus requested a review from Copilot April 22, 2025 21:50

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 water event handling by splitting the original WaterInfoEvent into separate events for water amount, mop attachment, and sweeping type. The changes include updating imports and replacing old event types in hardware modules and related capabilities, as well as introducing the new CapabilityWater abstraction.

Reviewed Changes

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

Show a summary per file
File Description
deebot_client/hardware/deebot/*.py Updated water event handling to use CapabilityWater and separate water_info events.
deebot_client/events/water_info.py Removed the combined WaterInfoEvent and introduced separate ValueEvent types.
deebot_client/commands/json/water_info.py Split aggregated water event notifications into individual events.
deebot_client/capabilities.py Replaced WaterInfoEvent usage with the new CapabilityWater type.
Other hardware and init files Updated corresponding imports and usage to align with the new event separation.
Comments suppressed due to low confidence (1)

deebot_client/commands/json/water_info.py:38

  • Ensure that tests are updated to verify that the split water events (WaterAmountEvent, MopAttachedEvent, and WaterSweepTypeEvent) are triggered correctly, reflecting the removal of the aggregated WaterInfoEvent.
event_bus.notify(WaterAmountEvent(WaterAmount(int(data["amount"]))));

@nanomad

nanomad commented Apr 22, 2025

Copy link
Copy Markdown
Contributor

LGTM, I'll try and rebase my Water commands and messages on top on this and see if it works

@codecov

codecov Bot commented Apr 22, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.46%. Comparing base (813daea) to head (f489d58).
Report is 2 commits behind head on dev.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #917   +/-   ##
=======================================
  Coverage   92.46%   92.46%           
=======================================
  Files         121      121           
  Lines        4644     4648    +4     
  Branches      292      292           
=======================================
+ Hits         4294     4298    +4     
  Misses        288      288           
  Partials       62       62           

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq

codspeed-hq Bot commented Apr 22, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #917 will not alter performance

Comparing split-WaterInfoEvent (f489d58) with dev (76b0d47)

Summary

✅ 6 untouched benchmarks

@nanomad

nanomad commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

@edenhaus works for me, I'll open a PR as soon as this is merged

@edenhaus edenhaus marked this pull request as ready for review April 25, 2025 20:44
@edenhaus edenhaus merged commit b24dc56 into dev Apr 25, 2025
27 checks passed
@edenhaus edenhaus deleted the split-WaterInfoEvent branch April 25, 2025 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: Breaking Change Pull request with braking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants