Add date-range FTP retrieval - #404
Conversation
WalkthroughThe PR adds a functional threshold power range API with date, sport, and aggregation validation. Lactate-threshold retrieval now uses this helper. Unit tests cover URL construction, date inputs, validation, and related range methods. ChangesThreshold power range APIs
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@garminconnect/__init__.py`:
- Around line 1442-1451: Update the start_date and end_date normalization logic
to handle datetime values before the generic date checks, converting them with
.date().isoformat() so the range URL always receives YYYY-MM-DD values. Preserve
existing date and string validation behavior, including the default for
end_date.
- Around line 1566-1571: In the functional-threshold calculation flow, move the
get_functional_threshold_power_range call before the speed and heart-rate
requests so inverted ranges are rejected by the helper before any Garmin API
call. Add a regression test covering an inverted start/end range and assert
connectapi is not called.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a196483a-bbe5-48c3-93c1-2dbb541cb1a7
📒 Files selected for processing (2)
garminconnect/__init__.pytests/test_garmin_unit.py
…gression test for it
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
garminconnect/__init__.py (1)
1564-1569: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize
datetimevalues before delegation.At Line 1564,
start_dateandend_datecan already be timestamp strings because the earlierisinstance(value, date)branches also matchdatetime. The helper then receives"YYYY-MM-DDTHH:MM:SS"instead of a date-only value. Handledatetimebeforedateinget_lactate_threshold, as the helper does. Add a regression test withdatetimeinputs.Proposed fix
- if isinstance(start_date, date): + if isinstance(start_date, datetime): + start_date = start_date.date().isoformat() + elif isinstance(start_date, date): start_date = start_date.isoformat() else: start_date = _validate_date_format(start_date, "start_date") - if isinstance(end_date, date): + if isinstance(end_date, datetime): + end_date = end_date.date().isoformat() + elif isinstance(end_date, date): end_date = end_date.isoformat()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@garminconnect/__init__.py` around lines 1564 - 1569, Update get_lactate_threshold so datetime inputs are normalized to date-only values before the existing date handling and delegation to get_functional_threshold_power_range; check datetime before date because datetime is a date subclass. Add a regression test covering datetime start_date and end_date inputs and verifying the helper receives normalized date values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@garminconnect/__init__.py`:
- Around line 1571-1573: Wrap the lactate threshold URL expressions assigned to
speed_url and heart_rate_url across adjacent f-strings so each line complies
with PEP 8 line-length conventions, without changing the resulting URLs.
---
Duplicate comments:
In `@garminconnect/__init__.py`:
- Around line 1564-1569: Update get_lactate_threshold so datetime inputs are
normalized to date-only values before the existing date handling and delegation
to get_functional_threshold_power_range; check datetime before date because
datetime is a date subclass. Add a regression test covering datetime start_date
and end_date inputs and verifying the helper receives normalized date values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 667e47fe-07cf-4d4e-b5b6-07b69daac41a
📒 Files selected for processing (2)
garminconnect/__init__.pytests/test_garmin_unit.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
garminconnect/__init__.py (1)
1564-1569:⚠️ Potential issue | 🟠 MajorNormalize
datetimeinputs before delegation.
datetimeis a subclass ofdate. The caller therefore converts adatetimetoYYYY-MM-DDTHH:MM:SSbefore callingget_functional_threshold_power_range. This bypasses the helper'sdatetimehandling and sends a non-date string to its date validator.Handle
datetimebeforedatefor both bounds. Add a regression test forstart_dateandend_datedatetime values.Proposed fix
- if isinstance(start_date, date): + if isinstance(start_date, datetime): + start_date = start_date.date().isoformat() + elif isinstance(start_date, date): start_date = start_date.isoformat() - if isinstance(end_date, date): + if isinstance(end_date, datetime): + end_date = end_date.date().isoformat() + elif isinstance(end_date, date): end_date = end_date.isoformat()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@garminconnect/__init__.py` around lines 1564 - 1569, Update the caller surrounding get_functional_threshold_power_range to detect and normalize datetime values before the broader date handling for both start_date and end_date, preserving datetime semantics so the delegated helper receives valid date inputs. Add a regression test covering datetime values for both bounds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@garminconnect/__init__.py`:
- Around line 1564-1569: Update the caller surrounding
get_functional_threshold_power_range to detect and normalize datetime values
before the broader date handling for both start_date and end_date, preserving
datetime semantics so the delegated helper receives valid date inputs. Add a
regression test covering datetime values for both bounds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2bc90593-b8d9-4ee7-bef7-9119a41ec600
📒 Files selected for processing (1)
garminconnect/__init__.py
|
Thanks a lot Martina! I will adapt the start and end params to the rest of the code and use the helpers to check date format... |
2 participants
Summary
get_cycling_ftp()returns only the latest cycling FTP value, whileget_lactate_threshold(latest=False)already calls Garmin's undocumented historical FTP range endpoint inline. That makes FTP history unavailable as a direct public API and duplicates the endpoint construction.This PR adds a range-capable sibling:
get_functional_threshold_power_range(start_date, end_date=None, *, sport="RUNNING", aggregation="daily")dateobjects;end_datedefaults to today/biometric-service/stats/functionalThresholdPower/range/{start}/{end}with Garmin's requiredsport,aggregation, andaggregationStrategy=LATESTparametersget_lactate_threshold(latest=False)now reuses this method for its power result, removing the duplicated endpoint construction without changing its returned structure or latest-value behavior.get_cycling_ftp()remains unchanged for callers that only need the current cycling FTP.Test plan
pdm run test: 215 passed, 18 integration tests deselecteddateobject inputs and non-default aggregationVerified against a live Garmin account.
Summary by CodeRabbit
New Features
Bug Fixes