feat: add clean record for Q7 #745

Lash-L · 2026-01-16T03:17:24Z

Would appreciate you being pretty in depth on this one as it is the second trait for Q7 so I want to identify anything now rather than later.

fixes #735

codecov · 2026-01-16T03:19:09Z

Codecov Report

❌ Patch coverage is 94.33962% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
roborock/data/b01_q7/b01_q7_containers.py	82.45%	10 Missing ⚠️
roborock/devices/traits/b01/q7/clean_summary.py	94.87%	1 Missing and 1 partial ⚠️

Files with missing lines	Coverage Δ
roborock/devices/traits/b01/q7/__init__.py	`95.45% <100.00%> (+0.71%)`	⬆️
tests/data/b01_q7/test_b01_q7_containers.py	`100.00% <100.00%> (ø)`
tests/devices/traits/b01/q7/__init__.py	`100.00% <100.00%> (ø)`
tests/devices/traits/b01/q7/conftest.py	`100.00% <100.00%> (ø)`
tests/devices/traits/b01/q7/test_clean_summary.py	`100.00% <100.00%> (ø)`
tests/devices/traits/b01/q7/test_init.py	`100.00% <100.00%> (ø)`
tests/mock_data.py	`100.00% <100.00%> (ø)`
roborock/devices/traits/b01/q7/clean_summary.py	`94.87% <94.87%> (ø)`
roborock/data/b01_q7/b01_q7_containers.py	`85.78% <82.45%> (-0.55%)`	⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR adds clean record functionality for B01 Q7 devices by implementing a new trait that retrieves and parses clean record data from the device's service.get_record_list API.

Changes:

Introduces four new data container classes (CleanRecordDetail, CleanRecordListItem, CleanRecordList, CleanRecordSummary) to represent clean record data structures
Implements CleanSummaryTrait for B01 Q7 devices with methods to fetch and parse clean records
Integrates the new trait into Q7PropertiesApi as a clean_summary attribute

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File	Description
roborock/data/b01_q7/b01_q7_containers.py	Adds data container classes for clean record information including detail fields, list items, and summary totals
roborock/devices/traits/b01/q7/clean_summary.py	Implements the CleanSummaryTrait with methods to fetch record lists, parse JSON detail fields, and retrieve clean record details
roborock/devices/traits/b01/q7/init.py	Integrates CleanSummaryTrait into Q7PropertiesApi and exports it in all
tests/data/b01_q7/test_b01_q7_containers.py	Adds test to verify CleanRecordList parsing and detail field extraction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

roborock/devices/traits/b01/q7/clean_summary.py

roborock/data/b01_q7/b01_q7_containers.py

roborock/devices/traits/b01/q7/clean_summary.py

roborock/data/b01_q7/b01_q7_containers.py

roborock/devices/traits/b01/q7/clean_summary.py

Copilot · 2026-01-16T03:23:36Z

tests/data/b01_q7/test_b01_q7_containers.py

+    detail = CleanSummaryTrait._parse_record_detail(parsed.record_list[0].detail)
+    assert isinstance(detail, CleanRecordDetail)
+    assert detail.method == 0
+    assert detail.clean_count == 1
+    assert detail.record_clean_way == 0
+    assert detail.record_faultcode == 0
+    assert detail.record_dust_num == 0
+    assert detail.clean_current_map == 0


The test appears to be missing assertions for some of the parsed detail fields. The test should verify record_start_time, record_use_time, record_clean_area, record_clean_mode, record_task_status, and record_map_url to ensure comprehensive coverage of the CleanRecordDetail parsing functionality.

roborock/devices/traits/b01/q7/clean_summary.py

allenporter · 2026-02-01T00:15:38Z

roborock/data/b01_q7/b01_q7_containers.py

+    """Represents an entry in the clean record list returned by `service.get_record_list`."""
+
+    url: str | None = None
+    detail: str | dict | None = None


This really can be either type?

I originally thought i saw data where it was not a str in the app code, but I looked again and I think it is always a json str.

So it's always a json dict? So then this should be dict (post json parsing)?

it is a dict after we parse from json - but not at the time of accessing/loading from result of api.

it is a dumped json dict within the main json dict, so it is not parsed.

We have to manually parse it before access. We don't update the value either after we parse - i guess we theoretically could?

CleanRecordList.from_dict(result) has detail as a str

then we access .detail -> parse it to a dict -> convert it to CleanRecordDetail

I don't think we should use the same field for the string or dict value and expose it in the API. Maybe there could be a manual step to "clean" the data first, converting it from json to dict, then parse it? or something else.

I have a local change i'm trying to finish up - but i changed the typing to be just str. You can let me know if that's what you were thinking

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Lash-L · 2026-02-01T03:41:46Z

tests/mock_data.py


 TESTDATA = pathlib.Path("tests/testdata")

-PRODUCTS = {file.name: json.load(file.open()) for file in TESTDATA.glob("home_data_product_*.json")}


unrelated change but windows seems to require this or tests crash.

I have just been doing it and not including it in any PRs

allenporter · 2026-02-01T19:21:46Z

tests/devices/traits/b01/q7/test_clean_summary.py

+        ],
+    }
+
+    details = await clean_summary_trait._get_clean_record_details(


Not sure about testing private methods like this

The alternative is I do a whole wrapping around the refresh() call, but this was a bit simplier mock wise. If you want me to change it i will.

I guess i assume this can be tested with either the response builder for a more end to end test, or with putting all the parsing inside the dataclass object and testing it there. I would probably just make last_record_detail also cacheable and test it entirely inside the dataclass, so everything is encapsulated.

allenporter · 2026-02-01T19:23:47Z

roborock/data/b01_q7/b01_q7_containers.py


    url: str | None = None
-    detail: str | dict | None = None
+    detail: str | None = None


Can you add a comment here explaining what's up with this field? It looks like its not really usable and folks are not expected to use this directly and should only access the summary for the last detail.

As an alternative you could put the json parsing here as something like detail_parsed in a @cachedproperty if you want to make it accessible and shared with the

add a cached property

allenporter

Fine to fix in follow up PRs since its just style

allenporter · 2026-02-01T21:23:51Z

roborock/devices/traits/b01/q7/clean_summary.py

-
+                parsed = item.detail_parsed
+            except RoborockException as ex:
+                # Rather than failing if something goes wrong here, we should fail and log to tell the user.


Given there is already an exception catch in detail_parsed i'd just push this in there? What I was trying to get at by suggesting moving the parsing into the dataclas is that this whole function feels like it can be a one-liner:

return reverse([ item.detail_parsed for item in record_list.record_list if item.detail_parsed is not None ])

While typically i think it is good to not swallow exceptions down deep, i'm just thinking since in practice you're ignoring parse errors then just make it less code.

allenporter · 2026-02-01T21:24:41Z

roborock/devices/traits/b01/q7/clean_summary.py

            raise TypeError(f"Unexpected response type for GET_RECORD_LIST: {type(result).__name__}: {result!r}")
        return CleanRecordList.from_dict(result)

    async def _get_clean_record_details(self, *, record_list: CleanRecordList) -> list[CleanRecordDetail]:


I don't think this needs to be async

allenporter · 2026-02-01T21:26:13Z

tests/devices/traits/b01/q7/test_clean_summary.py

+        ],
+    }
+
+    details = await clean_summary_trait._get_clean_record_details(


I guess i assume this can be tested with either the response builder for a more end to end test, or with putting all the parsing inside the dataclass object and testing it there. I would probably just make last_record_detail also cacheable and test it entirely inside the dataclass, so everything is encapsulated.

feat: add clean record for Q7

d4c0772

Lash-L requested review from allenporter and Copilot January 16, 2026 03:17

Copilot started reviewing on behalf of Lash-L January 16, 2026 03:17 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

allenporter requested changes Feb 1, 2026

View reviewed changes

Lash-L and others added 2 commits January 31, 2026 20:05

chore: some copilot suggestions

b27092e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

chore: testing and PR comments

1cd66b8

Lash-L commented Feb 1, 2026

View reviewed changes

Lash-L requested a review from allenporter February 1, 2026 03:42

allenporter previously approved these changes Feb 1, 2026

View reviewed changes

chore: move parsing to the container

1519e9b

Lash-L dismissed allenporter’s stale review via 1519e9b February 1, 2026 20:55

allenporter approved these changes Feb 1, 2026

View reviewed changes

Lash-L merged commit 329e52b into main Feb 1, 2026
8 checks passed


		TESTDATA = pathlib.Path("tests/testdata")

		PRODUCTS = {file.name: json.load(file.open()) for file in TESTDATA.glob("home_data_product_*.json")}

feat: add clean record for Q7 #745

feat: add clean record for Q7 #745

Uh oh!

Conversation

Lash-L commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allenporter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Lash-L commented Jan 16, 2026 •

edited

Loading

codecov bot commented Jan 16, 2026 •

edited

Loading