Conversation
Added None-checks in all Logger static methods to safely handle calls during Python interpreter shutdown when the Logger singleton may already be destroyed. Prevents AttributeError in __del__ destructors. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
When a channel's main list requires login and log_on() fails (e.g. user cancels device auth), return None from process_folder_list instead of an empty list. FolderAction detects the None sentinel and calls endOfDirectory(handle, False), causing Kodi to navigate back to the channel overview. A brief login-error notification is shown (matching the sub-item login failure pattern at line 500-508). Previously the login failure was silently ignored, leaving the user on an empty folder. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Container.Update triggers a separate plugin instance that races with endOfDirectory. Slow responses (e.g. token exchange, profile switch) cause Kodi to render the stale search-history page instead of the actual results. Run the search directly in the keyboard branch so results are rendered in the same plugin instance. Known issue: empty needle is passed through, so refreshing search results triggers the keyboard pop-up instead of re-running the query. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Empty folder results previously showed an Error notification regardless of the configured empty_list_behavior setting. Now only the 'error' behavior triggers an Error notification; all other modes use Info. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Without SORT_METHOD_UNSORTED as default, Kodi sorts alphabetically. This reorders pinned items (search, explore pages) meant to stay at fixed positions and shuffles live channel listings out of EPG order. Make SORT_METHOD_UNSORTED the default sort method when any pinned (dontGroup) or live items are present, matching the existing search behavior. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
The DRM warning dialog is unnecessary when inputstreamhelper confirms that InputStream Adaptive and the Widevine CDM are installed. Fall through to the existing warning if the helper is unavailable. This effectively makes the show_drm_paid_warning setting a no-op when Widevine is present. A future refactor could replace the setting entirely with a pop-up shown only when Widevine is not detected. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
The if/elif chain dispatched on body type first, with the HTTP method (PATCH) buried inside POST branches, making it harder to extend with new methods. Restructure to resolve the effective method up front, then dispatch with flat independent if blocks in natural HTTP order (GET, POST, PATCH). When no explicit method is passed but body arguments are present, the request is silently promoted from GET to POST. This preserves existing behavior relied on by ~20 callers. Ideally callers should specify the method explicitly and the handler should reject ambiguous requests. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
The HTTP request handler supported GET, POST, and PATCH but not DELETE. Add it to be more compliant with the HTTP specification. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
|
Big fat disclaimer, I am not a python coder, I know it very little. I will work more on it, but this was mostly vibe-coded. (do not shame me to much ;) I do want some early review happening however to see if the general approach is alright, and get some testing feedback from those who dare. I've been using an earlier version (where it would just stream live tv) for 2 weeks now and that part is great. (I haven't updated that on our TV yet as that was enough for the WAF for now :p) all other testing I did locally on my laptop and through the included (almost 100) tests of the test suite. I haven't looked at the test code almost at all however. |
|
Those are a lot of changes! Perhaps too much for a PR for NL Ziet. But anyways, good stuff. I do have some remarks, which I will post in the PR. One major this is that we should not include Python libs in other add-ons. So please see if you can use https://kodi.tv/addons/omega/script.module.qrcode/ for the QR Code stuff. Another thing is that I advice you to use https://pypi.org/project/sakee/. That is my Kodi emulator that you can use to run an add-on without Kodi and get some simple ASCII results. |
Most changes are scafolding changes needed for NLZIET, we can do those in seperate MR's to get into the framework, but then they'd have no user yet. Either way is fine. I've tried to split my commits into 'somewhat smallish' commits from a feature point of view, to avoid having 'one huge fully featured with everything' commit however.
So far as I could see, this is not an official addon, so we cannot depend on it, and will have to ask users to manually install it. There's other ways, with pure python implementations to do QR codes of course. But as I stated in the commit, the QR-code IS script.module.qrcode. So whatever approach you suggest we can do.
I've just ran whatever unittest.yaml did, which is installing the requirements.txt (which I would assume uses sakee?) and running the (full) pytest suite. |
|
P.s. if not already implied, try to review it on a per commit basis ;)
|
basrieter
left a comment
basrieter
left a comment
There was a problem hiding this comment.
Ok, looks like a lot of nice work! I will need to play around with it a bit, especially the device login.
For now I went through some of the (none-channel related) changes and left comments.
One thing (besides the QR stuff) is that you are making a lot of changes outside of adding NL Ziet. I understand these changes (some are nice, like the profile dependent search history). However, they clutter the PR and make it difficult to review and test.
So, even thought I understand your urge to update some 20 year old code, it would be better to separate this from the NL Ziet channel.
There was a problem hiding this comment.
I wonder if this will work, as we use Weblate to translate stuff. Retrospect is set up to use the en-gb as the main language and Weblate will them try to fill the rest. But we will see what happens.
There was a problem hiding this comment.
yeah, as a native dutch/german speaker I figured i'll add those 3 languages as well. I saw we used weblate for a lot of the other stuff; but I also got missing/empty strings when testing it on my system, so that was a little annoying :)
There was a problem hiding this comment.
So in general these tests are great. However....(there is always a but).....
I don't use tests for code testing at all (bare minimum) like you are doing here. These tests are really great stuff, but the tests in Retrospect are used to actually call API end-points and see if they still provide the API messages that work for Retrospect. So I abuse the unit tests to do integration testing. That is way the UriHandler is not mocked at all: I actually need it to do the http(s)-calls.
There was a problem hiding this comment.
yeah, I wasn't sure how to deal with that. I test both actually, but NLZIET requires a payed subscription. From early testing, after having an account, it may work without actually paying for it, just leaving the account 'on', so then we can still do some of the testing.
I didn't want to hammer the API, which is why I added both, real api tests for authentication, and mocking of everything else. This way, I could add a lot of different tests. There is one real API test that also validates the real API against the mocks.
Also, maybe more importantly, to be able to work on this one needs NLZIET credentials to do testing, which even account holders, may not want to do (locally), so being able to mock, allows for some basic tests to be performed. also mocking the API's at least gives us control what they will return, the real API will often return different content for certain endpoints of course. So I figured best of both worlds?
If the problem is the conftest thing, I can also make it part of only the NLZIET mocks. I just didn't know what the problem was and how to deal with it :)
There was a problem hiding this comment.
So basically what you do here, and in the other test, I usually combine. Because writing such extensive tests, just takes up too much time. My philosophy is, that if the API changes and breaks 2 things can happen:
- Parsing results on 0 results, or
- The converting from API to results fails
Both are covered in my simple tests like
def test_tv4_tv_shows(self):
url = "https://www.goplay.be/programmas/"
items = self._test_folder_url(url, 20)Which basically uses the TV4 channel (in this case) and just runs a parse off the given URL.
There was a problem hiding this comment.
I understand your philosophy, but I avoided it for the two previously mentioned reasons. a) other contributors may not have an account they want to use/sacrifice because of FUD on getting banned; being able to run tests locally without an internet connection :)
Having said that, I think it's trivial to do the same that we did with the authentication tests, run against the REAL API when username/password are set, against the MOCKS if not. I think doing mocks doesn't add a lot, I'll update this sunday as well.
tests/conftest.py
Outdated
There was a problem hiding this comment.
So this basically disables the UriHandler for all tests and it breaks all tests. So this won't work for Retrospect as we need actual https responses.
There was a problem hiding this comment.
yeah, fair enough; as mentioned above. i'll fix it sunday :)
There was a problem hiding this comment.
So, this file get's auto generated from the plugin.video.retrospect/resources/data/settings_template.xml. So the only thing you need to do is to configure your settings in the chn_nlziet.json, clear Python caches and run Retrospect once. It will generate the correct settings.xml.
There was a problem hiding this comment.
ahh, good to know, I wasn't sure how this worked, or rather, was confused by the committed settings.xml. Why are there settings committed? Should I commit them at all?
There was a problem hiding this comment.
Please don't update the urihandler.py. It is one of the core modules and should contain all stuff you need. If you really need something, please make the impact as minimal as possible.
There was a problem hiding this comment.
Please use the RETROSPECT_STDOUT_LOGGING to disable logging, not via these changes.
There was a problem hiding this comment.
these are old debug commits; as I said, I still need to review most of the code changes :p
|
I also noticed that if you have a PVR add-on installed, the pop-up for NLZiet appears, even when NPO Start is active: 
|
resources/lib/deviceauthdialog.py
Outdated
| return None | ||
|
|
||
|
|
||
| class DeviceAuthDialog(xbmcgui.WindowDialog): |
There was a problem hiding this comment.
I don't like mixing the xbmcgui.WindowDialog in Kodi plugin add-ons. It just looks ugly and never matches the actual skin users use. So I dialogs should be the most simple ones, where the skinning is part of the main skin, not Retrospect.
Can it not work with xbmc.executebuiltin('ShowPicture("https://example.com/image.jpg")')?
There was a problem hiding this comment.
I have no idea :D i was presented with three options, either the programmatically, like now, the standard dialog which did not allow for the needed modification of adding a QR code, or using a 'XML based dialog' which meant adding a skin, which I didn't want to do. I was under the impression that the programmatical gui would just use the skin, I was wrong, so I'll try this more, but will the showpicture work ontop of a dialog box? idk.
There was a problem hiding this comment.
So after digging deeper into this XML based dialog, the changes make a lot of sense to use that. I refrained from using it as we didn't have XML dialogs yet, but I also see it's quite trivial and works with skins, not against it. I'll fix that asap.
Every change presented to the framework, was done to support NLZIET (with device flow ;p) the profile dependent search history is truly just a 'nice to have' ;) So what do you recommend? just open single MR's for the trivial stuff, do you want to cherry pick some of them into master when you think they are good? Anything is fine by me; but the gross of the changes are needed for this new channel to work reliably. |
interesting, this I will have to check why this is, right now, i think the pop-up comes when the channel gets loaded and we detect no configured session, not sure if it's related to what you mention though. I'll have to think what would work best, only when entering the NLZIET channel list and we're not being authenticated. |
Channels that support user profiles (e.g. kids, adult, named) need isolated search histories so queries from one profile do not appear in another. Add a search_profile_id property to Channel (returns None by default). When a channel overrides it, SearchAction uses 'search:<profile_id>' as the settings key instead of the plain 'search' key. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Implemented complete OAuth2 authentication for NLZIET channel supporting both headless login (PKCE flow) and device flow (RFC 8628) for TV devices. Key features: - Headless browser-based username/password login - Session detection to skip login when already authenticated - Device flow for devices (nlziet.nl/koppel) - Automatic token refresh with silent re-authentication - Comprehensive test suite with AUTO/MANUAL user/pass authentication Based on OAuth2 implementation [0] by ArjenD and dut-iptv. [0]: https://github.com/dut-sees/kodi-nlziet Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Minimal NLZIET channel implementation: - Channel registration with dynamic channel discovery - Username/password authentication - Device flow authentication - Automatic profile selection after login (auto-selects if single profile) Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Add optional QR code display to DeviceAuthDialog for device flow authentication. When a caller supplies the qr_url parameter, a QR code for that parameter is inserted into the dialog box. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Adds live TV channel listing with DASH/Widevine playback, VOD categories (Movies, TV Shows, Trending, Recommended), Continue Watching, My List (watchlist), series/season/episode lists, and search functionality. Rich metadata: descriptions, poster/landscape artwork, channel logos, studio labels, broadcast dates, availability windows, and expiration dates. Items marked unavailable are filtered out. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Add up to three playable shortcuts before the season folders: Continue Watching (resume-aware), Most Recent Episode, and First Episode. When Continue Watching points to the same episode as First Episode (i.e. the series has not been started), the Continue item is omitted. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Implements create_iptv_streams() and create_iptv_epg() for IPTV Manager support. Provides all NLZIET live channels and 6 days of EPG data (3 past + 3 future). Replay-enabled programs include direct playback links in the EPG grid. Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl> Signed-off-by: Your Name <you@example.com>
Populate tv_show_title on episode MediaItems so service.upnext can show
the correct series title in its 'Up Next' popup.
The series title is extracted from the /v8/series/{id} detail response
in the existing extract_series_data() preprocessor and stored in each
season FolderItem's metaData under 'nlziet:series_title'.
A new extract_series_title() preprocessor on the /v9/series/ parser
reads that key from self.parentItem.metaData and caches it on the
channel instance. create_episode_item() then uses it to set
item.tv_show_title, which is forwarded to service.upnext via the
existing VideoAction.__get_up_next_data() framework method.
No new framework code is required; the trigger logic in videoaction.py
already fires automatically for any non-live episode when service.upnext
is installed.
Co-authored-by: Claude Sonnet 4.6
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Your Name <you@example.com>
Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
Co-authored-by: Anthropic Claude AI Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
|
Great work! I was looking for this for so long! 😀 So I'm happy to test. Iptv manager is not working yet right? Would be nice to use it to have some catchup options and watch stuff back from the epg. Is that something you're working on? Also, which branch should I test? The review version? Or the other one. |
- start_device_flow(): re-raise OSError (catches requests.ConnectionError, socket.gaierror, etc.) so callers can distinguish network failures from other errors. Non-OSError exceptions still return None as before. - __run_device_flow(): wrap start_device_flow() — OSError shows #30615 'Could not connect to service' instead of #30620 'Failed to start device setup', which was misleading when the real cause was no internet. - DeviceAuthDialog: replace onControl(control)/control.getId() with the standard Kodi idiom onClick(controlId) (integer dispatch). - DeviceAuthDialog: add threading.Event stop_event, set unconditionally in onClosed() before the safety-net logic. Fires for every close path: button click, Escape/Back, programmatic close(), success, timeout. - __poll_with_progress(): replace monitor.waitForAbort(0.5) with dialog.stop_event.wait(0.5) so the poll thread wakes immediately when the dialog closes rather than sleeping through its 0.5 s tick. Kodi abort is still checked via monitor.abortRequested() after the wait. - DeviceAuthDialog.xml: add <onclick>Close()</onclick> to the Cancel button — standard Kodi UX convention. Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Your Name <you@example.com>
- Shift visit-instruction, URL, enter-code, and user-code labels down 64 px (+64 from top=166→230, 208→272, 250→314, 292→356) to create roughly two visual lines of breathing room between the QR instruction text and the URL/code section. - Replace hardcoded texturenofocus (0xFF404040) with <texturenofocus /> (no background when unfocused) — the idiomatic Kodi pattern used in skin.estuary/Custom_1100_AddonLauncher.xml; eliminates the hardcoded hex value entirely. texturefocus keeps the named button_focus colour. - Pair texturenofocus / with textcolor=grey + focusedcolor=white so: unfocused state: grey label text, no background (readable on light bg) focused state: white label text on button_focus coloured rectangle Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Your Name <you@example.com>
The 2-of-3 majority vote in __pick_boundary_episode failed when broadcastAt timestamps were corrupted by re-broadcasts (De Luizenmoeder S1) or bulk imports (Flikken Maastricht S1). With firstAvailable always empty, the vote degraded to a 1-vs-1 tie that fell back to broadcastAt — exactly the wrong signal. Replace the vote with a simpler strategy: trust API order (always correct) and use broadcastAt monotonicity only to detect ascending vs descending direction. Non-monotonic or absent timestamps default to descending (the most common ordering across the NLZIET catalogue). Changes: - __fetch_season_episodes: return 2-tuples (broadcastAt, item) instead of 3-tuples, since firstAvailable is always empty. - __pick_boundary_episode: replaced vote logic with direction detection. - Added __is_ascending static method for broadcastAt monotonicity check. - Updated existing tests for new tuple format. - Added 15 new unit tests covering descending, ascending, corrupted broadcastAt (Flikken M. S1), re-broadcasts (De Luizenmoeder S1), single/empty lists, and __is_ascending edge cases. Co-authored-by: Claude Opus 4.6 Signed-off-by: Oliver Bramley <oliver@bramley.dev> Signed-off-by: Your Name <you@example.com>
For each algorithm test in TestEpisodeBoundaryDetection there is now a
corresponding integration test in TestNLZietChannelLive (auto-inherited
by TestNLZietChannelMocked so it runs without credentials).
Tests cover the three key real-world orderings from HAR recordings:
- Flikken Maastricht S18: descending, clean broadcastAt (A1 / A13)
- De Luizenmoeder S1: descending, non-monotonic re-broadcast dates (A1 / A10)
- Fawlty Towers S1: ascending, clean broadcastAt (A1 / A6)
Changes:
- nlziet_mocks.py: Added three HAR-derived episode fixtures (FM S18,
Luizenmoeder S1, Fawlty S1) and MOCK_EPISODES_BY_SEASON lookup dict;
extended dispatch() to handle /v9/series/{id}/episodes endpoint.
- test_chn_nlziet.py: Added 6 boundary tests to TestNLZietChannelLive
(_boundary_eps helper + first/recent assertion for each fixture).
Co-authored-by: Claude Sonnet 4.6
Signed-off-by: Oliver Bramley <oliver@bramley.dev>
Signed-off-by: Your Name <you@example.com>
Kodi generates settings.xml from the template only when a channel is first installed (no __pycache__). Since chn_nlziet.json was added after the initial channel discovery, the settings were never included. Commit the generated settings.xml directly so the 7 NLZIET channel settings are available immediately without requiring a fresh install: - nlziet_device_setup (device flow setup action) - nlziet_username / nlziet_password / nlziet_password_set - nlziet_log_off (logout action) - nlziet_select_profile - nlziet_live_start_offset Visibility indices for all other channels are shifted accordingly. Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Oliver Bramley <oliver@bramley.dev> Signed-off-by: Your Name <you@example.com>
- Switch from broken qrcode.image.pure (requires pymaging) to qrcode.make() default which uses PilImage when PIL is available - Generate QR PNG in set_content() (before onInit) to keep UI thread fast - Remove <visible>false</visible> from XML; manage all control visibility exclusively from Python onInit() per Kodi best practice - Hide QR text label when no qr_url is set (was showing blank) - Adjust QR image top position to sit just above the progress bar Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Your Name <you@example.com>
- Auth: initialise _time_since_poll = _interval so the first device-flow
poll fires within ~0.5 s instead of waiting a full interval (~5 s).
Approval is detected almost immediately after the user scans the QR code.
- Live stream big-skip navigation (DiagnosticPlayer / DiagnosticMonitor):
* onAVStarted detects NLZIET live streams by URL pattern (nlziet-linear).
* onPlayBackSeek redirects big backward skips (>4 min) to seekTime(0)
(start of 2-hour timeshift buffer) and big forward skips (>4 min) to
seekTime(getTotalTime()-5) (live edge).
* DiagnosticMonitor.onNotification intercepts Player.OnSeek for blocked
forward seeks at the live edge (inputstream.adaptive silently rejects
these, so onPlayBackSeek never fires, but the JSON-RPC notification
carries seekoffset and still arrives).
* A 2-second re-entry guard (_redirect_until) prevents the redirect
seekTime() call from triggering a second redirect.
* DiagnosticMonitor now receives a player reference via __init__
(player is created before monitor to satisfy the dependency).
* onPlayBackStopped / onPlayBackEnded / onPlayBackError all clear
_nlziet_live so the overrides do not bleed into the next stream.
Co-authored-by: Claude Sonnet 4.6
Signed-off-by: Oliver Schillings <o.schillings@outlook.com>
Signed-off-by: Your Name <you@example.com>
- Add xbmcgui.Dialog().notification() before seekTime() calls in
DiagnosticPlayer and DiagnosticMonitor so the user sees a toast
("Naar live" / "Naar begin uitzending") even when the buffer jump
is subtle (fresh stream with little history).
- Fix nlziet_password setting type: type="password" was dropped in
Kodi Nexus (20) and generated a parse warning on every startup.
Changed to type="text" -- the vault handles encryption on disk.
Co-authored-by: Claude Sonnet 4.6
Signed-off-by: Your Name <you@example.com>
When a live start offset is configured (> 0), inject the inputstream.adaptive.live_offset property onto the resolved stream. ISA uses this value in GetChapterPos() so that |< seeks to the post-ads position instead of the raw buffer start (second 0). Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Oliver Knieps <ok@example.com> Signed-off-by: Your Name <you@example.com>
Change type from 'number' to 'slider' with option='int' and range='-60,10,300' so Kodi enforces min/max bounds visually and won't accept out-of-range values (which previously caused CSettingsManager to return None, yielding start_offset=0). Co-authored-by: Claude Sonnet 4.6 Signed-off-by: Your Name <you@example.com>
|
3 participants
Functional description
This merge requests adds support for NLZIET for both live streams, VOD, profile support with device flow and username/password authentication.
Reasoning
Closes #645
Technical description
A new channel was added, and some plumbing fixed.
Tasks & Activities