Conversation
🔒 Dependency Security Report✅ No dependency changes detected in this PR. |
|
Hello! I have updated this provider starting with the great work that @OzGav did. I was going to use a seperate API library, but decided to use the local api_client.py that was already created.
Overall this should be a usable provider with basic sync functions. |
|
This will be reviewed fully in due course however a couple of things. I wont be the code owner. You can remove the status.md file. You need to add icon.svg and icon_monochrome.svg |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please address the copilot suggestiions and resolve them with a comment. |
|
Please apply
|
|
Marking this PR as draft so we can keep track of which PRs needs our attention. Please mark as 'Ready for review' when you want us to have another look 🙏 . |
Fixes: - Bug music-assistant#1: Replace self.lookup_key with self.instance_id - Episodes were failing to convert due to missing attribute - Changed lines 169, 174, 215 in __init__.py - Tested and verified working Documentation: - Add STATUS.md with complete development tracking - Implementation status (24 features implemented) - Testing results from 2026-02-01 - Bug documentation (1 fixed, 1 resolved, 1 investigating) - API endpoints reference - Contributing guidelines - Changelog Testing: - Verified login, podcast list, episodes work correctly - Identified resume position issue for further investigation - Documented all findings in STATUS.md Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Episodes were always starting at 0:00 instead of resuming from the last played position. Investigation revealed two separate issues: 1. get_resume_position() was returning seconds, but Music Assistant expects milliseconds 2. StreamDetails was missing the allow_seek=True flag, which prevented ffmpeg from applying the -ss seek parameter Both issues have been fixed and tested. Episodes now correctly resume from their saved positions. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updated STATUS.md with extensive documentation of Pocketcasts API capabilities: - Mapped 7 Pocketcasts features to Music Assistant provider features (RECOMMENDATIONS, FAVORITE_PODCASTS_EDIT, PLAYLIST features, LYRICS, etc.) - Replaced all guessed API endpoints with verified endpoints from unofficial Pocketcasts API documentation - Documented 20+ additional API endpoints organized by domain and functionality - Created phased implementation priority plan (Phase 1-4) - Cleaned up duplicate Library Management items (subscribe/unsubscribe) - Added notes on unconfirmed features (Transcripts, Filters) This provides a clear roadmap for future Pocketcasts provider development. Reference: https://github.com/yfhyou/api_pocketcasts/blob/main/reference/endpoints.md Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Added comprehensive documentation about Pocketcasts authentication tokens: Authentication Findings: - Pocketcasts API supports two token types: 1. Mobile/API tokens (scope: "mobile") - valid ~5 months, no refresh needed 2. Web player tokens (scope: "webplayer") - valid 1 hour, requires refresh - Current implementation uses mobile tokens (long-lived) - Token refresh is not required for our use case Documentation Updates: - Added "Authentication & Token Management" section explaining token types - Moved token refresh to Phase 4 (low priority) with rationale - Clarified that /user/token endpoint is for web player refresh only - Added explanation of JWT claims (iat, exp) in mobile tokens - Marked login error handling as tested and working This explains why the provider continues working after the supposed "1 hour expiry" - we're using mobile tokens that last months, not the short-lived web player tokens. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implemented 5 special browse folders that appear at root level: - Up Next: User's queued episodes - New Releases: Recent episodes from subscriptions - In Progress: Episodes currently being listened to - Starred: Favorited episodes - History: Recently played episodes API Client Changes: - Added get_up_next_episodes() for POST /up_next/list * Returns dict with UUIDs as keys (unique response format) - Added get_new_releases() for POST /user/new_releases - Added get_starred_episodes() for POST /user/starred - Added get_history() for POST /user/history - Fixed type hints in __aexit__ for proper async context manager Provider Changes: - Added _create_browse_folders() helper to generate folder list - Added _get_special_folder_episodes() to fetch folder-specific episodes - Updated browse() to show folders at root and handle folder paths - Special handling for Up Next endpoint's unique response format: * Returns episodes as dict with UUIDs as keys (not a list) * Modified iteration to handle both dict and list formats * Extract episode UUID from dict key when missing from data * Handle podcast field as both string (Up Next) and object (others) Testing: - All 5 browse folders tested and working - Episodes display correctly in all folders - Playback works from all folder types Documentation: - Updated STATUS.md with implementation details - Added changelog entry for 2026-02-02 - Documented special handling for Up Next endpoint - Updated test results and next steps Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add complete library management functionality allowing users to subscribe and unsubscribe from podcasts directly through Music Assistant, syncing changes back to Pocketcasts account. API Client Changes: - Add subscribe_podcast() method (POST /user/podcast/subscribe) - Add unsubscribe_podcast() method (POST /user/podcast/unsubscribe) - Enhanced error logging with response status and reason Provider Changes: - Implement library_add() for subscribing to podcasts - Implement library_remove() for unsubscribing from podcasts - Both methods delegate to base class for non-podcast media types - Full error handling and logging Testing: - Verified subscribe/unsubscribe work correctly in UI - Tested API error handling with invalid UUIDs - API validates UUID format (400 for malformed) - API accepts well-formed UUIDs even if non-existent (200) - Not a concern in practice - UUIDs come from Pocketcasts APIs Documentation: - Mark LIBRARY_PODCASTS_EDIT as fully implemented - Clarify FAVORITE_PODCASTS_EDIT not applicable to Pocketcasts - Document API behavior regarding UUID validation - Add comprehensive testing notes Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add on_played() callback for syncing playback progress every 30 seconds - Mark episodes as played when within 45 seconds of actual end - Use /user/episode endpoint for accurate duration (fixes early completion) - Add episode to Up Next via play_now() when starting playback - Archive completed episodes and remove from Up Next queue - Unarchive episodes when replaying from earlier position - Ignore MA's fully_played flag (uses wrong duration from static API) API methods added: - get_episode_details() - Fetch real duration and playback status - mark_episode_played() - Mark episode as played (status=3) - mark_episode_unplayed() - Reset to unplayed (status=1) - archive_episode() - Archive/unarchive episodes - remove_from_up_next() - Remove from Up Next queue - play_now() - Add to Up Next at top position Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reduced from 868 to 362 lines by consolidating redundant sections: - Merged feature status sections into single Feature Status table - Combined potential/priority/not-implemented into Roadmap section - Simplified known issues and testing status into compact tables - Condensed changelog while preserving key information Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add provider icons (icon.svg, icon_monochrome.svg), fix bugs found in Copilot code review: correct provider field in _convert_podcast to use instance_id, add missing media_type to StreamDetails, fix str(None) bug in config handling, add exception chaining in get_podcast, cache episode duration in on_played to reduce API calls. Also remove STATUS.md from PR, fix manifest name to "Pocket Casts", remove duplicate import and unused PLAY_URL constant, and remove verbose search response logging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
API client methods now raise PocketCastsAPIError on non-200 responses and LoginError on 401/403, instead of silently returning empty lists. This lets callers distinguish between "empty library" and "API failure", and ensures handle_async_init fails fast on auth problems. Added try/except to _get_special_folder_episodes, the one unprotected caller. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add SVG browse folder icons (up next, new releases, in progress, starred, history) matching the Pocket Casts web player style - Embed icons as base64 data URIs for self-contained operation - Add remotely_accessible=True to podcast thumbnail MediaItemImage - Refactor _create_browse_folders to use a loop with icon lookup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace get_podcast_episodes() + linear scan with get_episode_details() which fetches a single episode directly via /user/episode. This avoids downloading the entire episode list on every playback start. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Normalize position (treat None as 0) since MA's mark_item_played allows seconds_played=None. Gate the "mark unplayed" branch on not is_playing to avoid conflating playback at position 0 with the explicit unplayed action. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inject the shared MA http_session into PocketCastsClient via constructor instead of creating and managing a standalone aiohttp.ClientSession. This is consistent with other providers and reuses the shared connector, User-Agent, and response class. Remove session ownership (context manager, close in unload) since the session lifecycle is managed by MA. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Call /history/do when an episode begins playing to add it to the Pocket Casts listening history, alongside the existing play_now call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace duplicated 0.9 threshold and special folder name literals with FULLY_PLAYED_THRESHOLD and SPECIAL_FOLDERS module-level constants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All other API methods raise LoginError on 401/403 responses. Add the same check to get_podcast_episodes to match the established pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both get_podcast_episodes and browse contained identical logic for applying in-progress and history playback status to episode objects. Extract this into _enrich_episode_with_status to ensure both paths stay in sync and the browse path now also gets the debug logging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Move api_client import above TYPE_CHECKING block - Move FULLY_PLAYED_THRESHOLD and SPECIAL_FOLDERS before SUPPORTED_FEATURES - Fix codeowners format in manifest.json Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Caches search (7 days), get_podcast (24h), and get_podcast_episode (1h) to avoid hammering the Pocket Casts API on repeated requests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was added in eb326d2. I hope it is as you intended! |
| class PocketCastsAPIError(Exception): | ||
| """Base exception for Pocket Casts API errors.""" | ||
|
|
||
|
|
||
| class LoginError(PocketCastsAPIError): | ||
| """Login failed.""" |
There was a problem hiding this comment.
You need to use the MA errors directly or raise aiohttp.ClientError-derived exceptions and let the provider translate them
|
|
||
| return success | ||
|
|
||
| except Exception as err: |
There was a problem hiding this comment.
Wherever you have this catch-all it needs to be narrowed to the specific error you are looking for.
| LOGGER.info("Found %d podcasts for query '%s'", len(podcasts), query) | ||
| return podcasts | ||
|
|
||
| async def get_podcast_details(self, podcast_uuid: str) -> dict[str, Any] | None: |
There was a problem hiding this comment.
This is unusual as it is trying two endpoints in sequence regardless of whether the first succeeded with valid data? And the second endpoint appears to send json=None?
There was a problem hiding this comment.
Furthermore, this function does not seem to get called at all?
| LOGGER.debug("Retrieved %d in-progress episodes", len(episodes)) | ||
| return episodes | ||
|
|
||
| async def get_up_next_episodes(self) -> dict[str, dict[str, Any]] | list[dict[str, Any]]: |
There was a problem hiding this comment.
The return type should be normalised here to avoid the caller having to do work
| @@ -0,0 +1,589 @@ | |||
| """Simple Pocket Casts API client built from scratch.""" | |||
There was a problem hiding this comment.
The read methods have no try/except for network errors — a connection failure will raise an unhandled aiohttp.ClientError all the way up. The provider doesn't handle it, (it uses bare except Exception), so it falls through to unhelpful logging.
|
|
||
| try: | ||
| # This endpoint returns a 302 redirect to the static JSON with timestamp | ||
| async with self.mass.http_session.get( |
There was a problem hiding this comment.
Why are you bypassing the api_client here?
| allow_seek=True, | ||
| ) | ||
|
|
||
| @use_cache(3600 * 24 * 7) |
There was a problem hiding this comment.
This seems long for podcasts which can change daily
|
|
||
| except Exception as err: | ||
| LOGGER.exception("Error adding podcast to library: %s", err) | ||
| return False |
There was a problem hiding this comment.
Need to raise. Same for next function
| LOGGER.exception("Failed to initialize: %s", err) | ||
| raise LoginFailed(f"Failed to initialize Pocket Casts: {err}") from err | ||
|
|
||
| async def unload(self, is_removed: bool = False) -> None: |
There was a problem hiding this comment.
Why the override which does nothing?
| # Get the podcast UUID from the item | ||
| podcast_uuid = item.item_id | ||
|
|
||
| LOGGER.info("Adding podcast to library: %s (%s)", item.name, podcast_uuid) |
There was a problem hiding this comment.
All these infos are going to spam the log. If they are necessary at all they should be at debug level.
| "documentation": "https://music-assistant.io/music-providers/pocketcasts/", | ||
| "type": "music", | ||
| "requirements": [], | ||
| "codeowners": "[@yfhyou]", |
There was a problem hiding this comment.
This should be an array, so ["yfhyou"]
MarvinSchenkel
left a comment
There was a problem hiding this comment.
Please have a look at our comments. Marking this PR as draft , feel free to mark it as 'ready for review' again when you want us to have another look
| LOGGER.info("Found %d podcasts for query '%s'", len(podcasts), query) | ||
| return podcasts | ||
|
|
||
| async def get_podcast_details(self, podcast_uuid: str) -> dict[str, Any] | None: |
There was a problem hiding this comment.
Furthermore, this function does not seem to get called at all?
Summary
Adds a new music provider for Pocketcasts podcast service, allowing users to:
Features
LIBRARY_PODCASTSLIBRARY_PODCASTS_EDITBROWSESEARCHImplementation
manifest.json- Provider configurationapi_client.py- Custom API client for Pocketcasts (reverse-engineered API)__init__.py- Main provider implementationSTATUS.md- Development documentationPlayback Sync
Test plan
Notes
🤖 Generated with Claude Code