Return calibration points together with map #423
Conversation
philek
force-pushed
the
feature/calibration
branch
2 times, most recently
from
686e02e to
3d6884d
Compare
philek
force-pushed
the
feature/calibration
branch
from
3d6884d to
9e11681
Compare
| @@ -597,6 +628,10 @@ def get_svg_map(self) -> str | None: | |||
| ), | |||
| ) | |||
|
|
|||
| # Build the SVG elements | |||
| svg_map = svg.SVG(width=manipulation.x.svg_max, height=manipulation.y.svg_max) | |||
There was a problem hiding this comment.
Why is these changed needed?
There was a problem hiding this comment.
When the SVG image has no size it makes it difficult (or impossible) for the Vacuum Map card to translate the vacuum coordinates back to the map after the image gets scaled again in the Home Assistant UI.
For example in my case the viewport size is 229x237, but Chrome reports the naturalWidth/naturalHeight as 145x150. I believe height is always 150 and width is proportional. Firefox returns 0 for both naturalWidth and naturalHeight. The viewport size is not readily available.
However, if we set the width and height explicitly for the SVG then its available as naturalWidth/naturalHeight and the coordinate mapping works correctly.
There was a problem hiding this comment.
We should not set a width or height, as the front end should decide how big it should be drawn.
The problem with setting a width/height on an SVG is that the frontend will use it for showing. For that reason, we use the viewport.
More info about this topic can be found in home-assistant/core#98036 (comment)
We are calculating the calibration points against the viewport and it's the card's job to apply the difference between the viewport and the actual width/height which the card decided to display.
@PiotrMachowski How much work is it to calculate the position of the object according to the viewport if an SVG only has a viewport and no width/height?
There was a problem hiding this comment.
@edenhaus I haven't tried using the map card with SVG yet, I'll have to check it out.
| self._last_image = str(svg_map) | ||
| points = tuple( | ||
| CalibrationPoint(point, manipulation) | ||
| for point in (Point(0, 0), Point(0, 100000), Point(100000, 0)) |
There was a problem hiding this comment.
Why do we need the points at 100000? Can we not use the map width and height as points?
If yes we could remove your changes to _calc_unbounded_point and _calc_point
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #423 +/- ##
==========================================
+ Coverage 83.19% 83.21% +0.02%
==========================================
Files 71 74 +3
Lines 2868 2997 +129
Branches 511 535 +24
==========================================
+ Hits 2386 2494 +108
- Misses 434 449 +15
- Partials 48 54 +6 ☔ View full report in Codecov by Sentry. |
|
Replaced by #433 |
I would like to take a stab at adding the calibration points to support the Vacuum Map card from this issue:
DeebotUniverse/Deebot-4-Home-Assistant#321
First step is to calculate such points in the client library.
My Python experience is very limited, so let me know if you would like me to refactor it somehow.