Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return calibration points together with map #423

Closed
wants to merge 3 commits into from
Closed

Return calibration points together with map #423

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6616a10
Add calibration points to map
Jan 21, 2024
9e11681
Set image size
Jan 22, 2024
a76299f
Add calibration points to map - refactor
Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 58 additions & 10 deletions deebot_client/map.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,30 @@
image: bytes


@dataclasses.dataclass(frozen=True)
class CalibrationPoint:
"""Calibration point."""

vacuum: Point
map: Point = dataclasses.field(init=False)
manipulation: dataclasses.InitVar[MapManipulation]

def __post_init__(self, manipulation: MapManipulation) -> None:
object.__setattr__(
self,
"map",
_calc_unbounded_point(self.vacuum.x, self.vacuum.y, manipulation),

Check warning on line 200 in deebot_client/map.py

View check run for this annotation

Codecov / codecov/patch

deebot_client/map.py#L200 

Added line #L200 was not covered by tests
)


@dataclasses.dataclass(frozen=True)
class CalibratedMap:
"""Map image with calibration points."""

calibration_points: tuple[CalibrationPoint, ...]
image: str


# SVG definitions referred by map elements
_SVG_DEFS = svg.Defs(
elements=[
Expand Down Expand Up @@ -262,18 +286,16 @@
(float(value) / _PIXEL_WIDTH) + _OFFSET - axis_manipulation.map_shift
)
new_value = axis_manipulation.transform(new_value)
# return value inside min and max
return round(
min(axis_manipulation.svg_max, max(0, new_value)), _ROUND_TO_DIGITS
)

return round(new_value, _ROUND_TO_DIGITS)

except (ZeroDivisionError, ValueError):
pass

return 0


def _calc_point(
def _calc_unbounded_point(
x: float,
y: float,
map_manipulation: MapManipulation,
Expand All @@ -284,6 +306,19 @@
)


def _calc_point(
x: float,
y: float,
map_manipulation: MapManipulation,
) -> Point:
unbounded = _calc_unbounded_point(x, y, map_manipulation)

return Point(
min(map_manipulation.x.svg_max, max(0, unbounded.x)),
min(map_manipulation.y.svg_max, max(0, unbounded.y)),
)


def _points_to_svg_path(
points: Sequence[Point | TracePoint],
) -> list[svg.PathData]:
Expand Down Expand Up @@ -390,7 +425,7 @@

self._map_data: Final[MapData] = MapData(event_bus)
self._amount_rooms: int = 0
self._last_image: str | None = None
self._last_image: CalibratedMap | None = None

Check warning on line 428 in deebot_client/map.py

View check run for this annotation

Codecov / codecov/patch

deebot_client/map.py#L428 

Added line #L428 was not covered by tests
self._unsubscribers: list[Callable[[], None]] = []

async def on_map_set(event: MapSetEvent) -> None:
Expand Down Expand Up @@ -564,6 +599,13 @@
)

def get_svg_map(self) -> str | None:
"""Return map as SVG string and set of calibration points."""

Check warning on line 602 in deebot_client/map.py

View check run for this annotation

Codecov / codecov/patch

deebot_client/map.py#L602 

Added line #L602 was not covered by tests
map = self.get_calibrated_map()
if map is None:

Check warning on line 604 in deebot_client/map.py

View check run for this annotation

Codecov / codecov/patch

deebot_client/map.py#L604 

Added line #L604 was not covered by tests
return None
return map.image

Check warning on line 607 in deebot_client/map.py

View check run for this annotation

Codecov / codecov/patch

deebot_client/map.py#L606-L607 

Added lines #L606 - L607 were not covered by tests
def get_calibrated_map(self) -> CalibratedMap | None:
"""Return map as SVG string."""
if not self._unsubscribers:
raise MapError("Please enable the map first")
Expand All @@ -582,9 +624,6 @@
self._last_image = None
return None

# Build the SVG elements
svg_map = svg.SVG()
svg_map.elements = [_SVG_DEFS]
manipulation = MapManipulation(
AxisManipulation(
map_shift=background.bounding_box[0],
Expand All @@ -597,6 +636,10 @@
),
)

# Build the SVG elements
svg_map = svg.SVG(width=manipulation.x.svg_max, height=manipulation.y.svg_max)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is these changed needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

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

However, if we set the width and height explicitly for the SVG then its available as naturalWidth/naturalHeight and the coordinate mapping works correctly.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link

Choose a reason for hiding this comment

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

@edenhaus I haven't tried using the map card with SVG yet, I'll have to check it out.

svg_map.elements = [_SVG_DEFS]

# Set map viewBox based on background map bounding box.
svg_map.viewBox = svg.ViewBoxSpec(
0,
Expand Down Expand Up @@ -637,7 +680,12 @@
_get_svg_positions(self._map_data.positions, manipulation)
)

self._last_image = str(svg_map)
points = tuple(
CalibrationPoint(point, manipulation)
for point in (Point(0, 0), Point(0, 100000), Point(100000, 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

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

)

self._last_image = CalibratedMap(calibration_points=points, image=str(svg_map))
_LOGGER.debug("[get_svg_map] Finish")
return self._last_image

Expand Down
Loading