diff --git a/deebot_client/map.py b/deebot_client/map.py index 54f6dc00b..17dc603fe 100644 --- a/deebot_client/map.py +++ b/deebot_client/map.py @@ -138,7 +138,8 @@ async def on_map_trace(event: MapTraceEvent) -> None: if event.start == 0: self._map_data.clear_trace_points() - self._map_data.add_trace_points(event.data) + if data := event.data.strip(): + self._map_data.add_trace_points(data) unsubscribers.append(self._event_bus.subscribe(MapTraceEvent, on_map_trace)) diff --git a/src/map.rs b/src/map.rs index 28aa3173c..0601b64fb 100644 --- a/src/map.rs +++ b/src/map.rs @@ -7,7 +7,7 @@ use base64::Engine; use byteorder::{LittleEndian, ReadBytesExt}; use crc32fast::Hasher; use image::{GenericImageView, GrayImage, Luma}; -use log::debug; +use log::{debug, error}; use once_cell::sync::Lazy; use png::{BitDepth, ColorType, Compression, Encoder}; use pyo3::exceptions::PyValueError; @@ -68,7 +68,7 @@ fn process_trace_points(trace_points: &[u8]) -> Result, Box Result, Box> { +fn extract_trace_points(value: &str) -> Result, Box> { let decompressed_data = decompress_base64_data(value)?; process_trace_points(&decompressed_data) } @@ -333,9 +333,11 @@ impl MapData { } fn add_trace_points(&mut self, value: String) -> Result<(), PyErr> { - self.trace_points.extend( - extract_trace_points(value).map_err(|err| PyValueError::new_err(err.to_string()))?, - ); + self.trace_points + .extend(extract_trace_points(&value).map_err(|err| { + error!("Failed to extract trace points: {};value:{}", err, value); + PyValueError::new_err(err.to_string()) + })?); Ok(()) } @@ -345,11 +347,21 @@ impl MapData { fn update_map_piece(&mut self, index: usize, base64_data: String) -> Result { if index >= self.map_pieces.len() { + error!( + "Index out of bounds; index:{}, base64_data:{}", + index, base64_data + ); return Err(PyValueError::new_err("Index out of bounds")); } self.map_pieces[index] - .update_points(base64_data) - .map_err(|err| PyValueError::new_err(err.to_string())) + .update_points(&base64_data) + .map_err(|err| { + error!( + "Failed to update map piece: {}; index:{}, base64_data:{}", + err, index, base64_data, + ); + PyValueError::new_err(err.to_string()) + }) } fn map_piece_crc32_indicates_update( @@ -358,6 +370,7 @@ impl MapData { crc32: u32, ) -> Result { if index >= self.map_pieces.len() { + error!("Index out of bounds; index:{}, crc32:{}", index, crc32); return Err(PyValueError::new_err("Index out of bounds")); } Ok(self.map_pieces[index].crc32_indicates_update(crc32)) @@ -627,7 +640,7 @@ impl MapPiece { self.pixels_indexed.as_ref() } - fn update_points(&mut self, base64_data: String) -> Result> { + fn update_points(&mut self, base64_data: &str) -> Result> { let decoded = decompress_base64_data(base64_data)?; let old_crc32 = self.crc32; @@ -795,7 +808,7 @@ mod tests { #[test] fn test_extract_trace_points_success() { let input = "XQAABACvAAAAAAAAAEINQkt4BfqEvt9Pow7YU9KWRVBcSBosIDAOtACCicHy+vmfexxcutQUhqkAPQlBawOeXo/VSrOqF7yhdJ1JPICUs3IhIebU62Qego0vdk8oObiLh3VY/PVkqQyvR4dHxUDzMhX7HAguZVn3yC17+cQ18N4kaydN3LfSUtV/zejrBM4="; - let result = extract_trace_points(input.to_string()).unwrap(); + let result = extract_trace_points(input).unwrap(); let expected = vec![ TracePoint { x: 0, @@ -985,9 +998,7 @@ mod tests { #[test] fn test_update_map_piece_of_empty_piece() { - let data = String::from( - "XQAABAAQJwAAAABv/f//o7f/Rz5IFXI5YVG4kijmo4YH+e7kHoLTL8U6PAFLsX7Jhrz0KgA=", - ); + let data = "XQAABAAQJwAAAABv/f//o7f/Rz5IFXI5YVG4kijmo4YH+e7kHoLTL8U6PAFLsX7Jhrz0KgA="; let mut map_piece = MapPiece { crc32: 0, pixels_indexed: None, diff --git a/src/util.rs b/src/util.rs index f5af2a9b9..61199c2aa 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,3 +1,4 @@ +use log::error; use std::error::Error; use std::io::{Cursor, Read}; @@ -7,7 +8,7 @@ use liblzma::stream::Stream; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; -pub fn decompress_base64_data(value: String) -> Result, Box> { +pub fn decompress_base64_data(value: &str) -> Result, Box> { let bytes = general_purpose::STANDARD.decode(value)?; if is_zstd_compressed(&bytes) { @@ -51,8 +52,11 @@ fn is_zstd_compressed(bytes: &[u8]) -> bool { /// Decompress base64 decoded compressed string by using lzma or zstd #[pyfunction(name = "decompress_base64_data")] -fn python_decompress_base64_data(value: String) -> Result, PyErr> { - decompress_base64_data(value).map_err(|err| PyValueError::new_err(err.to_string())) +fn python_decompress_base64_data(value: &str) -> Result, PyErr> { + decompress_base64_data(value).map_err(|err| { + error!("Error decompressing base64 data: {}; value:{}", err, value); + PyValueError::new_err(err.to_string()) + }) } pub fn init_module(m: &Bound<'_, PyModule>) -> PyResult<()> { diff --git a/tests/rs/test_map.py b/tests/rs/test_map.py new file mode 100644 index 000000000..23edfbcb9 --- /dev/null +++ b/tests/rs/test_map.py @@ -0,0 +1,92 @@ +"""Test rust map module.""" + +from __future__ import annotations + +import pytest +from testfixtures import LogCapture + +from deebot_client.rs.map import MapData + + +@pytest.mark.parametrize( + ("value", "expected_error", "expected_log"), + [ + ( + "invalid_base64", + "Invalid symbol 95, offset 7.", + "Failed to extract trace points: Invalid symbol 95, offset 7.;value:invalid_base64", + ), + ( + "", + "Invalid 7z compressed data", + "Failed to extract trace points: Invalid 7z compressed data;value:", + ), + ], +) +def test_MapData_add_trace_points_invalid( + value: str, expected_error: str, expected_log: str +) -> None: + """Test invalid MapData.add_trace_points.""" + map_data = MapData() + with pytest.raises(ValueError, match=expected_error), LogCapture() as log: + map_data.add_trace_points(value) + log.check_present( + ( + "deebot_client.map", + "ERROR", + expected_log, + ) + ) + + +@pytest.mark.parametrize( + ("index", "base64_data", "expected_error", "expected_log"), + [ + ( + 10000, + "invalid_base64", + "Index out of bounds", + "Index out of bounds; index:10000, base64_data:invalid_base64", + ), + ( + 1, + "invalid_base64", + "Invalid symbol 95, offset 7.", + "Failed to update map piece: Invalid symbol 95, offset 7.; index:1, base64_data:invalid_base64", + ), + ( + 1, + "", + "Invalid 7z compressed data", + "Failed to update map piece: Invalid 7z compressed data; index:1, base64_data:", + ), + ], +) +def test_MapData_update_map_piece_invalid( + index: int, base64_data: str, expected_error: str, expected_log: str +) -> None: + """Test invalid MapData.update_map_piece.""" + map_data = MapData() + with pytest.raises(ValueError, match=expected_error), LogCapture() as log: + map_data.update_map_piece(index, base64_data) + log.check_present( + ( + "deebot_client.map", + "ERROR", + expected_log, + ) + ) + + +def test_MapData_map_piece_crc32_indicates_update_invalid() -> None: + """Test invalid MapData.map_piece_crc32_indicates_update.""" + map_data = MapData() + with pytest.raises(ValueError, match="Index out of bounds"), LogCapture() as log: + map_data.map_piece_crc32_indicates_update(1000, 1) + log.check_present( + ( + "deebot_client.map", + "ERROR", + "Index out of bounds; index:1000, crc32:1", + ) + ) diff --git a/tests/test_map.py b/tests/test_map.py index 4ef7c357d..c111917e8 100644 --- a/tests/test_map.py +++ b/tests/test_map.py @@ -2,7 +2,7 @@ import asyncio from typing import TYPE_CHECKING -from unittest.mock import ANY, AsyncMock, Mock, call +from unittest.mock import ANY, AsyncMock, Mock, call, patch import pytest @@ -188,6 +188,19 @@ async def test_get_svg_map_empty( assert map.get_svg_map() is None +async def test_empty_maptrace( + execute_mock: AsyncMock, + event_bus: EventBus, + static_device_info: StaticDeviceInfo, +) -> None: + """Test empty data will not raise exception.""" + with patch("deebot_client.map.MapData", autospec=True): + map = await setup_map(execute_mock, event_bus, static_device_info) + event_bus.notify(MapTraceEvent(0, 0, "")) + await block_till_done(event_bus) + map._map_data.add_trace_points.assert_not_called() + + def test_get_svg_map( event_loop: asyncio.AbstractEventLoop, benchmark: BenchmarkFixture,