Fix empty data on MapTraceEvent and log input values of rust functions on error

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))
 

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<Vec<TracePoint>, Box<dyn
         .collect()
 }
 
-fn extract_trace_points(value: String) -> Result<Vec<TracePoint>, Box<dyn Error>> {
+fn extract_trace_points(value: &str) -> Result<Vec<TracePoint>, Box<dyn Error>> {
     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<bool, PyErr> {
         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<bool, PyErr> {
         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<bool, Box<dyn std::error::Error>> {
+    fn update_points(&mut self, base64_data: &str) -> Result<bool, Box<dyn std::error::Error>> {
         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,

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<Vec<u8>, Box<dyn Error>> {
+pub fn decompress_base64_data(value: &str) -> Result<Vec<u8>, Box<dyn Error>> {
     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<Vec<u8>, PyErr> {
-    decompress_base64_data(value).map_err(|err| PyValueError::new_err(err.to_string()))
+fn python_decompress_base64_data(value: &str) -> Result<Vec<u8>, 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<()> {

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",
+        )
+    )

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,