Refactor service event to distinguish event with data or without (#308)

Only charging events have data and some of them have the data optionally.
It could make sense to get only two types of events: with data or without.
Currenly only charging events will have these types, all other service
events will be of ServiceEvent base type.

This solution has pros and cons:
- this is simpler and shorter
- only two event types to distinguish and process
But:
- this solution is not future proof: if any of the event start to get
other data, it would be required to introduce new types anyway which
will make mqtt._parse_topic function to look even more ugly and
complicated than now.

NOTE!!! This change is not compatible with current one that is used in
HA addon. Adaptation is required.

Co-authored-by: WebSpider <[email protected]>

myskoda/models/service_event.py

@@ -6,9 +6,7 @@
 from typing import Generic, TypeVar
 
 from mashumaro import field_options
-from mashumaro.config import BaseConfig
 from mashumaro.mixins.orjson import DataClassORJSONMixin
-from mashumaro.types import Discriminator
 
 from .charging import ChargeMode, ChargingState
 
@@ -46,14 +44,6 @@ class ServiceEvent(Generic[T], DataClassORJSONMixin):
     Service Events are unsolicited events emitted by the MQTT bus towards the client.
     """
 
-    class Config(BaseConfig):
-        """Configuration class for altering the behavior of discriminator."""
-
-        discriminator = Discriminator(
-            field="name",
-            include_subtypes=True,
-        )
-
     version: int
     producer: str
     name: ServiceEventName
@@ -122,76 +112,11 @@ class ServiceEventChargingData(ServiceEventData):
     )
 
 
-class ServiceEventChangeAccess(ServiceEvent):
-    """Event class for change-access service event."""
-
-    name = ServiceEventName.CHANGE_ACCESS
-    data: ServiceEventData
-
-
-class ServiceEventChangeChargeMode(ServiceEvent):
-    """Event class for change-charge-mode service event."""
-
-    name = ServiceEventName.CHANGE_CHARGE_MODE
-    data: ServiceEventData
-
-
-class ServiceEventChangeLights(ServiceEvent):
-    """Event class for change-lights service event."""
-
-    name = ServiceEventName.CHANGE_LIGHTS
-    data: ServiceEventData
-
-
-class ServiceEventChangeRemainingTime(ServiceEvent):
-    """Event class for change-remaining-time service event."""
-
-    name = ServiceEventName.CHANGE_REMAINING_TIME
-    data: ServiceEventData
-
-
-class ServiceEventChangeSoc(ServiceEvent):
-    """Event class for change-soc service event."""
-
-    name = ServiceEventName.CHANGE_SOC
-    data: ServiceEventChargingData
-
-
-class ServiceEventChargingCompleted(ServiceEvent):
-    """Event class for charging-completed service event."""
-
-    name = ServiceEventName.CHARGING_COMPLETED
+@dataclass
+class ServiceEventWithChargingData(ServiceEvent):
     data: ServiceEventChargingData
 
 
-class ServiceEventChargingStatusChanged(ServiceEvent):
-    """Event class for charging-status-changed service event."""
-
-    name = ServiceEventName.CHARGING_STATUS_CHANGED
-    data: ServiceEventData
-
-
-class ServiceEventClimatisationCompleted(ServiceEvent):
-    """Event class for climatisation-completed service event."""
-
-    name = ServiceEventName.CLIMATISATION_COMPLETED
-    data: ServiceEventData
-
-
-class ServiceEventDepartureReady(ServiceEvent):
-    """Event class for depature-ready service event."""
-
-    name = ServiceEventName.DEPARTURE_READY
-    data: ServiceEventData
-
-
-class ServiceEventDepartureStatusChanged(ServiceEvent):
-    """Event class for depature-status-changed service event."""
-
-    name = ServiceEventName.DEPARTURE_STATUS_CHANGED
-    data: ServiceEventData
-
-
 class UnexpectedChargeModeError(Exception):
     pass
 

myskoda/mqtt.py

@@ -16,7 +16,7 @@
 import aiomqtt
 
 from myskoda.auth.authorization import Authorization
-from myskoda.models.service_event import ServiceEvent
+from myskoda.models.service_event import ServiceEvent, ServiceEventWithChargingData
 
 from .const import (
     MQTT_ACCOUNT_EVENT_TOPICS,
@@ -213,6 +213,14 @@ def _on_message(self, msg: aiomqtt.Message) -> None:
 
         self._parse_topic(topic_match, data)
 
+    @staticmethod
+    def _get_charging_event(data: str) -> ServiceEvent:
+        try:
+            event = ServiceEventWithChargingData.from_json(data)
+        except ValueError:
+            event = ServiceEvent.from_json(data)
+        return event
+
     def _parse_topic(self, topic_match: re.Match[str], data: str) -> None:
         """Parse the topic and extract relevant parts."""
         [user_id, vin, event_type, topic] = topic_match.groups()
@@ -263,7 +271,7 @@ def _parse_topic(self, topic_match: re.Match[str], data: str) -> None:
                         vin=vin,
                         user_id=user_id,
                         timestamp=datetime.now(tz=UTC),
-                        event=ServiceEvent.from_json(data),
+                        event=self._get_charging_event(data),
                     )
                 )
             elif event_type == EventType.SERVICE_EVENT and topic == "departure":

tests/test_mqtt.py

@@ -14,13 +14,11 @@
 from myskoda.models.charging import ChargeMode, ChargingState
 from myskoda.models.operation_request import OperationName, OperationRequest, OperationStatus
 from myskoda.models.service_event import (
-    ServiceEventChangeAccess,
-    ServiceEventChangeLights,
-    ServiceEventChangeSoc,
-    ServiceEventChargingCompleted,
+    ServiceEvent,
     ServiceEventChargingData,
     ServiceEventData,
     ServiceEventName,
+    ServiceEventWithChargingData,
 )
 from myskoda.myskoda import MySkoda
 
@@ -158,6 +156,22 @@ async def test_subscribe_event(
                 }
             ),
         ),
+        (
+            f"{base_topic}/service-event/charging",
+            json.dumps(
+                {
+                    "version": 1,
+                    "traceId": trace_id,
+                    "timestamp": timestamp_str,
+                    "producer": "SKODA_MHUB",
+                    "name": "charging-completed",
+                    "data": {
+                        "userId": USER_ID,
+                        "vin": VIN,
+                    },
+                }
+            ),
+        ),
     ]
 
     def on_event(event: Event) -> None:
@@ -190,7 +204,7 @@ def on_event(event: Event) -> None:
             vin=VIN,
             user_id=USER_ID,
             timestamp=ANY,
-            event=ServiceEventChangeLights(
+            event=ServiceEvent(
                 version=1,
                 trace_id=trace_id,
                 timestamp=timestamp,
@@ -203,7 +217,7 @@ def on_event(event: Event) -> None:
             vin=VIN,
             user_id=USER_ID,
             timestamp=ANY,
-            event=ServiceEventChangeAccess(
+            event=ServiceEvent(
                 version=1,
                 trace_id=trace_id,
                 timestamp=timestamp,
@@ -216,7 +230,7 @@ def on_event(event: Event) -> None:
             vin=VIN,
             user_id=USER_ID,
             timestamp=ANY,
-            event=ServiceEventChangeSoc(
+            event=ServiceEventWithChargingData(
                 version=1,
                 trace_id=trace_id,
                 timestamp=timestamp,
@@ -237,7 +251,7 @@ def on_event(event: Event) -> None:
             vin=VIN,
             user_id=USER_ID,
             timestamp=ANY,
-            event=ServiceEventChargingCompleted(
+            event=ServiceEventWithChargingData(
                 version=1,
                 trace_id=trace_id,
                 timestamp=timestamp,
@@ -254,4 +268,20 @@ def on_event(event: Event) -> None:
                 ),
             ),
         ),
+        EventCharging(
+            vin=VIN,
+            user_id=USER_ID,
+            timestamp=ANY,
+            event=ServiceEvent(
+                version=1,
+                trace_id=trace_id,
+                timestamp=timestamp,
+                producer="SKODA_MHUB",
+                name=ServiceEventName.CHARGING_COMPLETED,
+                data=ServiceEventData(
+                    user_id=USER_ID,
+                    vin=VIN,
+                ),
+            ),
+        ),
     ]

tests/test_service_event.py

@@ -15,6 +15,7 @@
     ServiceEventChargingData,
     ServiceEventData,
     ServiceEventName,
+    ServiceEventWithChargingData,
 )
 
 FIXTURES_DIR = Path(__file__).parent.joinpath("fixtures")
@@ -37,7 +38,10 @@ def load_service_events() -> list[str]:
 
 def test_parse_service_events(service_events: list[str]) -> None:
     for service_event in service_events:
-        event = ServiceEvent.from_json(service_event)
+        try:
+            event = ServiceEventWithChargingData.from_json(service_event)
+        except ValueError:
+            event = ServiceEvent.from_json(service_event)
 
         if event.name == ServiceEventName.CHANGE_SOC:
             assert event.data == ServiceEventChargingData(