Repository: bimmerconnected/bimmer_connected
Pull request #271 (2021-02-24 20:06:46)

Bugfixes for remote service and vehicle position
## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->
- Bugfixes for remote service and vehicle position (#269)
- Revert CRLF issues from #268 (#270)

## Type of change
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (which adds functionality to this library)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #
- This PR is related to issue: 

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [ ] The code change is tested and works locally.
- [ ] Tests have been added to verify that the new code works.



Patch:
diff --git a/bimmer_connected/country_selector.py b/bimmer_connected/country_selector.py
index 238b103..03aabee 100644
--- a/bimmer_connected/country_selector.py
+++ b/bimmer_connected/country_selector.py
@@ -45,7 +45,7 @@ _GCDM_OAUTH_AUTHORIZATION = {
 
 def valid_regions() -> List[str]:
     """Get list of valid regions as strings."""
-    return [k.lower() for k in Regions.__members__.keys()]
+    return [region.name.lower() for region in Regions]
 
 
 def get_region_from_name(name: str) -> Regions:
@@ -53,8 +53,8 @@ def get_region_from_name(name: str) -> Regions:
 
     This function is not case-sensitive.
     """
-    for region_name, region in Regions.__members__.items():
-        if name.lower() == region_name.lower():
+    for region in Regions:
+        if name.lower() == region.name.lower():
             return region
     raise ValueError(
         "Unknown region {}. Valid regions are: {}".format(
diff --git a/bimmer_connected/remote_services.py b/bimmer_connected/remote_services.py
index 5bd62f9..ced2d5d 100644
--- a/bimmer_connected/remote_services.py
+++ b/bimmer_connected/remote_services.py
@@ -116,9 +116,12 @@ class RemoteServiceStatus:  # pylint: disable=too-few-public-methods
 
     def __init__(self, response: dict):
         """Construct a new object from a dict."""
-        status = response['executionStatus']
+        status = response.get('executionStatus', {})
+        if len(status) == 0:
+            _LOGGER.warning("executionStatus not in response: %s", response)
+
         self.state = ExecutionState(status.get('status', 'UNKNOWN'))
-        self.event_id = status['eventId']
+        self.event_id = status.get('eventId')
 
     @staticmethod
     def _parse_timestamp(timestamp: str) -> datetime.datetime:
diff --git a/bimmer_connected/vehicle_status.py b/bimmer_connected/vehicle_status.py
index 68ab3e2..fb1f2e9 100644
--- a/bimmer_connected/vehicle_status.py
+++ b/bimmer_connected/vehicle_status.py
@@ -139,6 +139,9 @@ class VehicleStatus:  # pylint: disable=too-many-public-methods
         if not self.is_vehicle_tracking_enabled:
             _LOGGER.warning('Vehicle tracking is disabled')
             return None
+        if self.is_vehicle_active:
+            _LOGGER.warning('Vehicle was moving at last update, no position available')
+            return None
         pos = self._state.attributes[SERVICE_STATUS]['position']
         return float(pos['lat']), float(pos['lon'])
 
@@ -152,9 +155,21 @@ class VehicleStatus:  # pylint: disable=too-many-public-methods
         if not self.is_vehicle_tracking_enabled:
             _LOGGER.warning('Vehicle tracking is disabled')
             return None
+        if self.is_vehicle_active:
+            _LOGGER.warning('Vehicle was moving at last update, no position available')
+            return None
         pos = self._state.attributes[SERVICE_STATUS]['position']
         return int(pos['heading'])
 
+    @property
+    @backend_parameter
+    def is_vehicle_active(self) -> bool:
+        """Check if the vehicle is active/moving.
+
+        If the vehicle was active/moving at the time of the last status update, current position is not available.
+        """
+        return self._state.attributes[SERVICE_STATUS]['position']['status'] in ['VEHICLE_ACTIVE']
+
     @property
     @backend_parameter
     def is_vehicle_tracking_enabled(self) -> bool:
diff --git a/pylintrc b/pylintrc
index a63f80b..8c376b7 100644
--- a/pylintrc
+++ b/pylintrc
@@ -1,2 +1,5 @@
 [FORMAT]
-max-line-length=120
\ No newline at end of file
+max-line-length=120
+
+[MESSAGES CONTROL]
+disable=R0801
\ No newline at end of file


Test patch:
diff --git a/test/responses/G31_NBTevo/status_position_vehicle_active.json b/test/responses/G31_NBTevo/status_position_vehicle_active.json
new file mode 100644
index 0000000..89c7585
--- /dev/null
+++ b/test/responses/G31_NBTevo/status_position_vehicle_active.json
@@ -0,0 +1,52 @@
+{
+  "vehicleStatus" : {
+    "doorPassengerFront" : "CLOSED",
+    "cbsData" : [ {
+      "cbsDueDate" : "2020-01",
+      "cbsState" : "OK",
+      "cbsRemainingMileage" : 25000,
+      "cbsDescription" : "Next service due when the stated distance has been covered or by the specified date.",
+      "cbsType" : "OIL"
+    }, {
+      "cbsDueDate" : "2022-01",
+      "cbsState" : "OK",
+      "cbsRemainingMileage" : 60000,
+      "cbsDescription" : "Next visual inspection due when the stated distance has been covered or by the stated date.",
+      "cbsType" : "VEHICLE_CHECK"
+    }, {
+      "cbsDueDate" : "2021-01",
+      "cbsState" : "OK",
+      "cbsDescription" : "Next change due at the latest by the stated date.",
+      "cbsType" : "BRAKE_FLUID"
+    } ],
+    "windowDriverFront" : "CLOSED",
+    "DCS_CCH_Ongoing" : false,
+    "updateReason" : "VEHICLE_SHUTDOWN_SECURED",
+    "rearWindow" : "CLOSED",
+    "remainingRangeFuelMls" : 187,
+    "DCS_CCH_Activation" : "NA",
+    "singleImmediateCharging" : false,
+    "positionLight" : "OFF",
+    "vin" : "G31_VIN",
+    "doorDriverFront" : "CLOSED",
+    "mileage" : 4134,
+    "checkControlMessages" : [ ],
+    "parkingLight" : "OFF",
+    "windowDriverRear" : "CLOSED",
+    "steering" : "LH",
+    "updateTime" : "2018-03-12T08:56:16+0100",
+    "remainingFuel" : 32,
+    "windowPassengerRear" : "CLOSED",
+    "trunk" : "CLOSED",
+    "hood" : "CLOSED",
+    "internalDataTimeUTC" : "2018-03-12T07:56:16",
+    "windowPassengerFront" : "CLOSED",
+    "doorLockState" : "SECURED",
+    "doorPassengerRear" : "CLOSED",
+    "remainingRangeFuel" : 302,
+    "doorDriverRear" : "CLOSED",
+    "position" : {
+      "status" : "VEHICLE_ACTIVE"
+    }
+  }
+}
\ No newline at end of file
diff --git a/test/test_vehicle_status.py b/test/test_vehicle_status.py
index d33d99e..5be0f76 100644
--- a/test/test_vehicle_status.py
+++ b/test/test_vehicle_status.py
@@ -13,6 +13,7 @@ from bimmer_connected.vehicle_status import LidState, LockState, ConditionBasedS
 
 G31_TEST_DATA = load_response_json('G31_NBTevo/status.json')
 G31_NO_POSITION_TEST_DATA = load_response_json('G31_NBTevo/status_position_disabled.json')
+G31_NO_POSTITION_VEHICLE_ACTIVE_TEST_DATA = load_response_json('G31_NBTevo/status_position_vehicle_active.json')
 F48_TEST_DATA = load_response_json('F48/status.json')
 I01_TEST_DATA = load_response_json('I01_REX/status.json')
 
@@ -61,7 +62,7 @@ class TestState(unittest.TestCase):
         self.assertFalse(state.vehicle_status.are_parking_lights_on)
         self.assertEqual(ParkingLightState.OFF, state.vehicle_status.parking_lights)
 
-    def test_parse_g31_no_psoition(self):
+    def test_parse_g31_no_position(self):
         """Test parsing of G31 data with position tracking disabled in the vehicle."""
         account = unittest.mock.MagicMock(ConnectedDriveAccount)
         state = VehicleState(account, None)
@@ -71,6 +72,17 @@ class TestState(unittest.TestCase):
         self.assertIsNone(state.vehicle_status.gps_position)
         self.assertIsNone(state.vehicle_status.gps_heading)
 
+    def test_parse_g31_no_position_vehicle_active(self):
+        """Test parsing of G31 data with vehicle beeing active."""
+        account = unittest.mock.MagicMock(ConnectedDriveAccount)
+        state = VehicleState(account, None)
+        state._attributes[SERVICE_STATUS] = G31_NO_POSTITION_VEHICLE_ACTIVE_TEST_DATA['vehicleStatus']
+
+        self.assertTrue(state.vehicle_status.is_vehicle_tracking_enabled)
+        self.assertTrue(state.vehicle_status.is_vehicle_active)
+        self.assertIsNone(state.vehicle_status.gps_position)
+        self.assertIsNone(state.vehicle_status.gps_heading)
+
     def test_parse_f48(self):
         """Test if the parsing of the attributes is working."""
         account = unittest.mock.MagicMock(ConnectedDriveAccount)