Add date-range variants of get_max_metrics/get_rhr_day/get_hrv_data/get_sleep_data - #402
Open
mannmann2 wants to merge 2 commits into
Open
Add date-range variants of get_max_metrics/get_rhr_day/get_hrv_data/get_sleep_data#402mannmann2 wants to merge 2 commits into
mannmann2 wants to merge 2 commits into
Conversation
get_max_metrics, get_rhr_day, get_hrv_data, and get_sleep_data all hit Garmin endpoints that natively support a date range, but the wrappers hardcode start == end == cdate. Add range-capable siblings that reuse the same URLs/params: - get_max_metrics_range(start, end) - get_hrv_data_range(start, end) - get_rhr_daily(start, end) — filters the wellness-stats metricsMap response down to WELLNESS_RESTING_HEART_RATE - get_sleep_daily(start, end) — chunks to the sleep-stats endpoint's 28-day cap, following the same pattern as get_daily_steps - get_calories_daily(start, end) — new method, reuses the same wellness-stats endpoint as get_rhr_daily with metricId 22/23 (active/BMR calories), which wasn't wrapped at all before Each avoids N+1 single-day calls when a caller wants history rather than a snapshot. Unit tests added for URL/param construction and sleep's chunking behavior.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughAdded five Garmin date-range methods for max metrics, sleep, resting heart rate, calories, and HRV. The implementation validates dates, queries range endpoints, normalizes responses, and adds unit tests for URL construction, filtering, aggregation, sorting, and chunking. ChangesGarmin wellness date-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 |
mannmann2
marked this pull request as ready for review
July 28, 2026 17:23
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 1404-1409: Wrap the public-method docstrings to follow PEP 8
line-length conventions. Update get_max_metrics_range and the RHR, calorie, and
HRV methods in garminconnect/__init__.py at lines 1404-1409, 1830-1836,
1857-1863, and 1910-1915 respectively; preserve their wording and behavior while
splitting overly long lines appropriately.
- Around line 1410-1412: Reject inverted date ranges consistently by adding and
reusing a shared ordered-range validator after the existing date normalization
in the max-metrics, RHR, calorie, and HRV methods; ensure validation occurs
before URL construction or API requests. In tests/test_garmin_unit.py lines
351-445, add parameterized reversed-range coverage for every variant and assert
that no API request is made.
🪄 Autofix (Beta)
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: df42a380-fd69-419a-a4e4-f32f0b7b4e68
📒 Files selected for processing (2)
garminconnect/__init__.pytests/test_garmin_unit.py
CodeRabbit caught that get_max_metrics_range, get_rhr_daily, get_calories_daily, and get_hrv_data_range validated date format but not ordering, silently sending a reversed fromDate/untilDate to Garmin instead of rejecting start > end (get_sleep_daily already had this check). Add a shared _validate_date_range() helper and use it in all five methods. Also wraps docstrings to stay under the line-length limit, and adds a parameterized regression test asserting each method rejects an inverted range without making an API call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Several single-day methods call Garmin endpoints that natively support a date range, but hardcode
fromDate = untilDate = cdate, forcing callers who want history into N single-day requests:get_max_metrics(cdate)→.../metrics-service/metrics/maxmet/daily/{cdate}/{cdate}get_rhr_day(cdate)→.../userstats-service/wellness/daily/{display_name}withfromDate=untilDate=cdateget_hrv_data(cdate)→.../hrv-service/hrv/{cdate}(range form lives at.../hrv-service/hrv/daily/{start}/{end})get_sleep_data(cdate)→ single-night detail only; no daily-summary-range equivalent existsThis PR adds range-capable siblings rather than changing existing signatures, so nothing breaks:
get_max_metrics_range(start, end)get_hrv_data_range(start, end)get_rhr_daily(start, end)— same wellness-stats endpoint asget_rhr_day, filtered down to theWELLNESS_RESTING_HEART_RATEseriesget_sleep_daily(start, end)— daily sleep summaries over a range, chunked to the sleep-stats endpoint's 28-day cap (same pattern already used byget_daily_stepsfor its own 28-day limit)get_calories_daily(start, end)— new method (no existing calories wrapper), reuses the same wellness-stats endpoint asget_rhr_dailywithmetricId=[22, 23]for active/BMR caloriesI verified the underlying endpoint paths/params against a live Garmin account before writing this (not just guessed from adjacent code), then re-confirmed each one against this repo's own already-merged patterns (
get_daily_steps's chunking,get_lactate_threshold's range-vs-latest split) to keep the style consistent.Test plan
pytest tests -m "not integration"— 201 passed (was 192, +9 new)ruff check/ruff format --checkclean on touched filesSummary by CodeRabbit
start/endinputs.