Attempt at multi-bike support #82
Conversation
|
CoMPaTech
changed the title
|
@coderabbitai review |
Actions PerformedReview triggered.
|
WalkthroughThe update introduces multibike support and a new button feature for resetting trip data in the Stromer Home Assistant integration. It enhances the configuration flow, adds new translations, and updates the core logic to support these functionalities. The version has been incremented, and debug logging has been improved. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant HomeAssistant
participant StromerIntegration
participant StromerAPI
User->>HomeAssistant: Configure Stromer Integration
HomeAssistant->>StromerIntegration: async_step_user
StromerIntegration->>StromerAPI: Validate Input
StromerAPI-->>StromerIntegration: Return Bike Data
StromerIntegration->>User: Select Bike
User->>StromerIntegration: Select Bike
StromerIntegration->>HomeAssistant: Create Entry
User->>HomeAssistant: Press Reset Trip Data Button
HomeAssistant->>StromerIntegration: async_press
StromerIntegration->>StromerAPI: Reset Trip Data
StromerAPI-->>StromerIntegration: Confirm Reset
StromerIntegration-->>HomeAssistant: Update State
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
custom_components/stromer/stromer.py (1)
Line range hint
225-239: The methodstromer_call_apiis well-implemented for fetching data from the API. Consider using format specifiers for string formatting for consistency and readability.- log = "API call status: %s" % res.status - log = "API call returns: %s" % ret + log = f"API call status: {res.status}" + log = f"API call returns: {ret}"
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- CHANGELOG.md (1 hunks)
- README.md (1 hunks)
- custom_components/stromer/init.py (2 hunks)
- custom_components/stromer/button.py (1 hunks)
- custom_components/stromer/config_flow.py (4 hunks)
- custom_components/stromer/const.py (1 hunks)
- custom_components/stromer/manifest.json (1 hunks)
- custom_components/stromer/strings.json (1 hunks)
- custom_components/stromer/stromer.py (6 hunks)
- custom_components/stromer/translations/en.json (1 hunks)
- custom_components/stromer/translations/nl.json (1 hunks)
Files skipped from review due to trivial changes (4)
- CHANGELOG.md
- custom_components/stromer/const.py
- custom_components/stromer/manifest.json
- custom_components/stromer/translations/nl.json
Additional Context Used
Ruff (9)
custom_components/stromer/stromer.py (9)
129-129: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling
189-189: Use format specifiers instead of percent format
191-191: Use format specifiers instead of percent format
205-205: Use format specifiers instead of percent format
207-207: Use format specifiers instead of percent format
220-220: Use format specifiers instead of percent format
222-222: Use format specifiers instead of percent format
234-234: Use format specifiers instead of percent format
236-236: Use format specifiers instead of percent format
Additional comments not posted (13)
custom_components/stromer/strings.json (1)
26-30: The addition of the "Reset trip data" button entity is correctly implemented and aligns with the new feature requirements.custom_components/stromer/translations/en.json (1)
26-30: The English translation for the "Reset trip data" button is correctly implemented.custom_components/stromer/button.py (4)
4-8: The imports are well-organized and correctly scoped for the functionality required in this file.Also applies to: 13-15
17-24: The BUTTONS tuple is correctly defined with appropriate attributes for the "Reset trip data" button.
26-37: Theasync_setup_entryfunction is properly updated to handle the dynamic addition of button entities based on the bike data.
40-67: TheStromerButtonclass is well-implemented with clear and concise logic for handling button presses and entity setup.custom_components/stromer/__init__.py (2)
19-19: The addition ofPlatform.BUTTONto the PLATFORMS list is appropriate for supporting the new button functionality.
39-41: The handling of bike-specific data in theasync_setup_entryfunction is correctly implemented, enhancing the configuration capabilities for individual bikes.custom_components/stromer/config_flow.py (2)
Line range hint
27-42: Thevalidate_inputfunction is correctly updated to handle bike data validation, crucial for the multi-bike support feature.
50-79: Theasync_step_bikemethod is well-implemented, providing a clear and user-friendly flow for bike selection during configuration.README.md (1)
38-39: The README is updated to include information about the new "Reset trip data" button, providing clear guidance to users on the new feature.custom_components/stromer/stromer.py (2)
3-3: Updated version number to "0.2.0" aligns with the PR's intent to introduce significant functionality.
39-39: Initialization offull_data,bike_id,bike_name, andbike_modelsupports the new multi-bike functionality.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- custom_components/stromer/stromer.py (6 hunks)
Additional Context Used
Ruff (9)
custom_components/stromer/stromer.py (9)
129-129: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling
189-189: Use format specifiers instead of percent format
191-191: Use format specifiers instead of percent format
205-205: Use format specifiers instead of percent format
207-207: Use format specifiers instead of percent format
220-220: Use format specifiers instead of percent format
222-222: Use format specifiers instead of percent format
233-233: Use format specifiers instead of percent format
235-235: Use format specifiers instead of percent format
Additional comments not posted (3)
custom_components/stromer/stromer.py (3)
3-3: Updated module version to "0.2.0" aligns with the PR's objective to introduce significant new features.
39-39: Initialization of new attributes for handling bike data is a good practice for managing state within the class.
237-238: The conditional return based on thefullflag is a good practice for API data fetching, allowing for flexible data retrieval.
| async def stromer_detect(self) -> dict: | ||
| """Get full data (to determine bike(s)).""" | ||
| try: | ||
| await self.stromer_update() | ||
| self.full_data = await self.stromer_call_api(endpoint="bike/", full=True) | ||
| except Exception as e: | ||
| log = f"Stromer unable to update: {e}" | ||
| log = f"Stromer unable to fetch full data: {e}" | ||
| LOGGER.error(log) | ||
|
|
||
| LOGGER.debug("Stromer connected!") | ||
|
|
||
| return self.status | ||
| log = f"Stromer full_data : {self.full_data}" | ||
| LOGGER.debug(log) | ||
| return self.full_data |
There was a problem hiding this comment.
Ensure proper error handling in stromer_detect by using raise ... from e to maintain the traceback.
- log = f"Stromer unable to fetch full data: {e}"
- LOGGER.error(log)
+ LOGGER.error(f"Stromer unable to fetch full data: {e}")
+ raise ApiError from eCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def stromer_detect(self) -> dict: | |
| """Get full data (to determine bike(s)).""" | |
| try: | |
| await self.stromer_update() | |
| self.full_data = await self.stromer_call_api(endpoint="bike/", full=True) | |
| except Exception as e: | |
| log = f"Stromer unable to update: {e}" | |
| log = f"Stromer unable to fetch full data: {e}" | |
| LOGGER.error(log) | |
| LOGGER.debug("Stromer connected!") | |
| return self.status | |
| log = f"Stromer full_data : {self.full_data}" | |
| LOGGER.debug(log) | |
| return self.full_data | |
| async def stromer_detect(self) -> dict: | |
| """Get full data (to determine bike(s)).""" | |
| try: | |
| self.full_data = await self.stromer_call_api(endpoint="bike/", full=True) | |
| except Exception as e: | |
| LOGGER.error(f"Stromer unable to fetch full data: {e}") | |
| raise ApiError from e | |
| log = f"Stromer full_data : {self.full_data}" | |
| LOGGER.debug(log) | |
| return self.full_data |
| # try: | ||
| # await self.stromer_update() | ||
| # except Exception as e: | ||
| # log = f"Stromer unable to update: {e}" | ||
| # LOGGER.error(log) | ||
|
|
||
| LOGGER.debug("Stromer connected!") | ||
|
|
||
| # return self.status | ||
| return True |
There was a problem hiding this comment.
The commented-out code should be removed if it's no longer needed, to keep the codebase clean and maintainable.
- # try:
- # await self.stromer_update()
- # except Exception as e:
- # log = f"Stromer unable to update: {e}"
- # LOGGER.error(log)
- # LOGGER.debug("Stromer connected!")
- # return self.status
- # return TrueCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # try: | |
| # await self.stromer_update() | |
| # except Exception as e: | |
| # log = f"Stromer unable to update: {e}" | |
| # LOGGER.error(log) | |
| LOGGER.debug("Stromer connected!") | |
| # return self.status | |
| return True | |
| LOGGER.debug("Stromer connected!") | |
| return True |
| async def stromer_reset_trip_data(self) -> None: | ||
| """Reset the trip data through the API.""" | ||
| endpoint = f"bike/id/{self.bike_id}/trip_data/" | ||
| url = f"{self.base_url}/rapi/mobile/v4.1/{endpoint}" | ||
| if self._api_version == "v3": | ||
| url = f"{self.base_url}/rapi/mobile/v2/{endpoint}" | ||
|
|
||
| headers = {"Authorization": f"Bearer {self._token}"} | ||
| res = await self._websession.delete(url, headers=headers) | ||
| ret = json.loads(await res.text()) | ||
| log = "API call trip data reset status: %s" % res.status | ||
| LOGGER.debug(log) | ||
| log = "API call trip data reset returns: %s" % ret | ||
| LOGGER.debug(log) | ||
| async def stromer_call_api(self, endpoint: str, full=False) -> Any: |
There was a problem hiding this comment.
Method stromer_reset_trip_data correctly implements the API call to reset trip data. However, consider using format specifiers for logging.
- log = "API call trip data reset status: %s" % res.status
- LOGGER.debug(log)
- log = "API call trip data reset returns: %s" % ret
+ log = f"API call trip data reset status: {res.status}"
+ LOGGER.debug(log)
+ log = f"API call trip data reset returns: {ret}"Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def stromer_reset_trip_data(self) -> None: | |
| """Reset the trip data through the API.""" | |
| endpoint = f"bike/id/{self.bike_id}/trip_data/" | |
| url = f"{self.base_url}/rapi/mobile/v4.1/{endpoint}" | |
| if self._api_version == "v3": | |
| url = f"{self.base_url}/rapi/mobile/v2/{endpoint}" | |
| headers = {"Authorization": f"Bearer {self._token}"} | |
| res = await self._websession.delete(url, headers=headers) | |
| ret = json.loads(await res.text()) | |
| log = "API call trip data reset status: %s" % res.status | |
| LOGGER.debug(log) | |
| log = "API call trip data reset returns: %s" % ret | |
| LOGGER.debug(log) | |
| async def stromer_call_api(self, endpoint: str, full=False) -> Any: | |
| async def stromer_reset_trip_data(self) -> None: | |
| """Reset the trip data through the API.""" | |
| endpoint = f"bike/id/{self.bike_id}/trip_data/" | |
| url = f"{self.base_url}/rapi/mobile/v4.1/{endpoint}" | |
| if self._api_version == "v3": | |
| url = f"{self.base_url}/rapi/mobile/v2/{endpoint}" | |
| headers = {"Authorization": f"Bearer {self._token}"} | |
| res = await self._websession.delete(url, headers=headers) | |
| ret = json.loads(await res.text()) | |
| log = f"API call trip data reset status: {res.status}" | |
| LOGGER.debug(log) | |
| log = f"API call trip data reset returns: {ret}" | |
| LOGGER.debug(log) | |
| async def stromer_call_api(self, endpoint: str, full=False) -> Any: |
Summary by CodeRabbit
New Features
Enhancements
Version Updates