-
-
Notifications
You must be signed in to change notification settings - Fork 223
Add and remove gear functions #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@psdupvi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Warning
|
Cohort / File(s) | Summary |
---|---|
Gear URL tweaks & Gear-Activity methodsgarminconnect/__init__.py |
Removed trailing slash from gear base URL and ensured subpaths use leading slashes; added add_gear_to_activity(gearUUID: str, activity_id: int | str) and remove_gear_from_activity(gearUUID: str, activity_id: int | str) which validate activity_id , build link/unlink URLs and perform PUT requests returning JSON. |
Demo: interactive flowdemo.py |
Added add_and_remove_gear_to_activity(api: Garmin) that picks a gear and the most recent activity, calls the new add/remove methods (with user confirmation) and wired this function into the Gear menu and execution mapping. |
Docs / Menu updateREADME.md |
Updated Gear & Equipment method count from 6 → 8 in the demo/menu listing and API Coverage Statistics to reflect the two new public methods. |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor Client
participant Garmin
participant GarminAPI as "Garmin Gear API"
rect rgb(240,248,255)
note over Client,Garmin: Add gear to activity (new)
Client->>Garmin: add_gear_to_activity(gearUUID, activity_id)
Garmin->>Garmin: validate activity_id\nbuild /gear/{gearUUID}/link/activity/{activity_id}
Garmin->>GarminAPI: PUT /gear/{gearUUID}/link/activity/{activity_id}
alt success (2xx)
GarminAPI-->>Garmin: 2xx + JSON
Garmin-->>Client: JSON
else error
GarminAPI-->>Garmin: error
Garmin-->>Client: propagate/raise
end
end
rect rgb(245,255,240)
note over Client,Garmin: Remove gear from activity (new)
Client->>Garmin: remove_gear_from_activity(gearUUID, activity_id)
Garmin->>Garmin: validate activity_id\nbuild /gear/{gearUUID}/unlink/activity/{activity_id}
Garmin->>GarminAPI: PUT /gear/{gearUUID}/unlink/activity/{activity_id}
alt success (2xx)
GarminAPI-->>Garmin: 2xx + JSON
Garmin-->>Client: JSON
else error
GarminAPI-->>Garmin: error
Garmin-->>Client: propagate/raise
end
end
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
I hop along the endpoints fine,
I link your sneaker, bike, or spine.
A PUT to bind, a PUT to free,
Gear and activity — now synced with glee.
Thump-thump, I dance: all set and merry 🐇✨
Pre-merge checks and finishing touches
✅ Passed checks (5 passed)
Check name | Status | Explanation |
---|---|---|
Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
Title Check | ✅ Passed | The title succinctly and accurately conveys the main change—adding and removing gear functions—which aligns directly with the new methods introduced in the pull request. |
Linked Issues Check | ✅ Passed | The pull request fulfills issue #282 by providing the add_gear_to_activity and remove_gear_from_activity methods, updating the README, and extending the demo, thereby delivering the requested support for assigning and unassigning gear to activities. |
Out of Scope Changes Check | ✅ Passed | All code changes, including URL formatting refinements, documentation updates, and demo enhancements, are directly related to enabling gear assignment functionality and introduce no unrelated features. |
Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help
to get the list of available commands and usage tips.
…places where it is used. Intended to match other URLs, which don't have a trailing slash
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
garminconnect/__init__.py (2)
2028-2043
: Align naming with existing gear APIs, validate inputs, add debug log, and fix minor docstring grammar.
- Use gearUUID to match existing methods (e.g., get_gear_stats, get_gear_activities).
- Validate activity_id as positive int; ensure non-empty gear UUID.
- Add a debug log for traceability.
- Grammar: "though" → "through"; tighten docstring.
-def add_gear_to_activity(self, gear_uid: str, activity_id: int) -> dict[str, Any]: - """ - Associates gear with an activity. Requires a gear_uid and an activity_id - - Args: - gear_uid: UID for gear to add to activity. Findable though the get_gear function - activity_id: Integer ID for the activity to add the gear to - - Returns: - Dictionary containing information for the added gear - - """ - - url = f"{self.garmin_connect_gear_baseurl}link/{gear_uid}/activity/{activity_id}" - return self.garth.put("connectapi", url).json() +def add_gear_to_activity( + self, gearUUID: str, activity_id: int | str +) -> dict[str, Any]: + """ + Associate gear with an activity. + + Args: + gearUUID: UID of the gear to add (retrievable through get_gear()). + activity_id: Activity ID to add the gear to. + + Returns: + Dictionary containing information for the added gear. + """ + # Validate inputs + gearUUID = str(gearUUID).strip() + if not gearUUID: + raise ValueError("gearUUID cannot be empty") + activity_id = _validate_positive_integer(int(activity_id), "activity_id") + + logger.debug("Linking gear %s to activity %s", gearUUID, activity_id) + url = ( + f"{self.garmin_connect_gear_baseurl}link/{gearUUID}/activity/{activity_id}" + ) + return self.garth.put("connectapi", url).json()
2044-2059
: Wrap signature to satisfy Black, correct return docstring, validate inputs, and add debug log.
- The long signature likely triggers the Black formatter failure; wrap it.
- Fix docstring: it currently says “added gear” for a removal.
- Match naming with existing gear methods; validate inputs as above.
After applying, run: pdm run black --check . (and pdm run black to auto-fix formatting if needed).
-def remove_gear_from_activity(self, gear_uid: str, activity_id: int) -> dict[str, Any]: - """ - Removes gear from an activity. Requires a gear_uid and an activity_id - - Args: - gear_uid: UID for gear to remove from activity. Findable though the get_gear method. - activity_id: Integer ID for the activity to remove the gear from - - Returns: - Dictionary containing information for the added gear - - """ - - url = f"{self.garmin_connect_gear_baseurl}unlink/{gear_uid}/activity/{activity_id}" - return self.garth.put("connectapi", url).json() +def remove_gear_from_activity( + self, + gearUUID: str, + activity_id: int | str, +) -> dict[str, Any]: + """ + Remove gear from an activity. + + Args: + gearUUID: UID of the gear to remove (retrievable through get_gear()). + activity_id: Activity ID to remove the gear from. + + Returns: + Dictionary containing information about the removed gear. + """ + # Validate inputs + gearUUID = str(gearUUID).strip() + if not gearUUID: + raise ValueError("gearUUID cannot be empty") + activity_id = _validate_positive_integer(int(activity_id), "activity_id") + + logger.debug("Unlinking gear %s from activity %s", gearUUID, activity_id) + url = ( + f"{self.garmin_connect_gear_baseurl}unlink/{gearUUID}/activity/{activity_id}" + ) + return self.garth.put("connectapi", url).json()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
garminconnect/__init__.py
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
garminconnect/__init__.py
[error] 1-1: Black formatting check failed. Command 'pdm run black --check .' reported that 1 file would be reformatted. Run 'pdm run black' to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
demo.py
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
demo.py (1)
garminconnect/__init__.py (6)
Garmin
(94-2210)get_device_last_used
(1513-1519)get_gear
(1850-1855)get_activities
(1521-1555)add_gear_to_activity
(2028-2050)remove_gear_from_activity
(2052-2072)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
demo.py
Outdated
activity = api.get_activities(0, 1)[0] | ||
activity_id = activity.get("activityId") | ||
activity_name = activity.get("activityName") | ||
for gear in gear_list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent IndexError when no activities exist
Line 2409 indexes the result of api.get_activities(0, 1)
without checking for an empty list. On accounts with no recorded activities this raises IndexError
, aborting the new interactive flow. Please guard the response before indexing.
Apply this diff to handle the empty-result case:
- activity = api.get_activities(0, 1)[0]
+ activities = api.get_activities(0, 1) or []
+ if not activities:
+ print("ℹ️ No activities found")
+ return
+ activity = activities[0]
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.
activity = api.get_activities(0, 1)[0] | |
activity_id = activity.get("activityId") | |
activity_name = activity.get("activityName") | |
for gear in gear_list: | |
activities = api.get_activities(0, 1) or [] | |
if not activities: | |
print("ℹ️ No activities found") | |
return | |
activity = activities[0] | |
activity_id = activity.get("activityId") | |
activity_name = activity.get("activityName") | |
for gear in gear_list: |
🤖 Prompt for AI Agents
In demo.py around lines 2409 to 2412, the code indexes api.get_activities(0,
1)[0] without checking for an empty list which can raise IndexError on accounts
with no activities; update the code to first assign the result to a variable,
check if the list is empty (e.g., if not activities: handle gracefully by
returning, skipping the interactive flow, or logging a message), and only then
extract activity_id and activity_name from activities[0]; ensure downstream code
that expects activity variables is skipped or given safe defaults when no
activity exists.
Adding two functions that allow you to add and remove gear from an activity. Resolves #282
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Documentation