Add optional station-level parental controls - #604
Conversation
There was a problem hiding this comment.
First off, thanks for this. Parental controls have been on the wishlist for a while and I really appreciate you taking a swing at it, especially the honesty in the "Known issues" section about the web remote. The API/queue plumbing for the digit input is clean and follows the existing patterns nicely. Lots of good stuff here.
I do have some changes to request before I can pull this in. A few are design-level.
-
Use existing GUI rendering patterns
The bitmap font dict +_draw_rect/_draw_text/_write_ppmis a lot of code to own. The lock screen should reuse the overlay pattern FS42 already runs everywhere else. Take a look atfs42/overlay/now_playing.pyandfs42/overlay/ticker.py: those are PySide6/Qt widgets launched as multiprocessing subprocesses, andstation_player.pyalready drives them (run_now_playing, the ticker, NFOAgent.show_overlay). A PIN prompt is a perfect fit for that same pattern as a small Qt overlay process that you push digit state to, rather than rasterizing an image ourselves. That should cut ~150 lines and gets you real font rendering, sizing, themes and refresh handling for free. We already support that, plus python TK for the classic guide and the glfw rending library the OSD uses, so I want to avoid another GUI rendering method. -
Use the queue/status channel we already have, not new JSON files
This is my biggest one. The digit commands going over player_command_queue is exactly right, but then status comes back through a brand-newruntime/parental_controls.jsonfile that the API reads. We already have a player->server status channel (update_status_socket, surfaced at /player/status). I'd rather add an awaiting_pin field (or something like that) to that existing status payload than stand up the parallel file-based channel. One source of truth for "what's the player doing," and no new file to reason about. -
The .ppm refresh files leak
This looks like a real bug:_show_parental_prompt_textcopies the image to a uniqueruntime/parental_controls_{count}_{ms}.ppmon every digit and every clear, but nothing ever deletes them. On a long-running box that's going to quietly pile up in runtime/. Moving to the Qt overlay (from 1 above) should make this disappear on its own, but flagging it anyway. -
Fit the PIN prompt into the state machine instead of a new inner loop
The main loop is really just a state machine — it dispatches on network_type, each play method (show_guide/show_web/play_slot) blocks and polls input_check until something happens, returns a PlayerOutcome, and the loop dispatches on that to pick the next state. So blocking itself is fine; that's how all of these work. My issue is that _prompt_for_parental_pin stands up its own parallel polling loop with another sleep(0.05) + input_check + payload-matching, instead of being expressed as a state the machine already understands. I'd rather see the prompt modeled as part of that flow (a PlayerState the play method returns and main_loop handles, or reuse the existing input handling) so we're not maintaining a second hand-rolled loop that has to stay in sync with the others. -
Code style - go lighter on the narrative comments.
A lot of the new comments narrate what the next line does rather than why e.g. # Dialog box, similar to a simple 90s/00s password prompt., # Refresh the active station config before any runtime station checks., # Title, # PIN boxes, #Divider. The code already says what it's doing, comments actually make the code harder to read unless they are explaining a non-obvious why or a gotcha. Mind trimming these down to match the rest of the codebase, which stays pretty sparse? Same goes for docstrings that just restate the method name.
None of this is a knock on the idea, I want the feature. Mostly its about leaning on what FS42 already gives you with the Qt overlay subprocesses in fs42/overlay/ for the screen, use the main loop state machine and the existing command queue + status socket for talking between the server and the player. Without the parallel machinery, this gets a lot smaller and easier to maintain. Happy to point you at the right spots in now_playing.py/ticker.py and the status-socket code if that'd help. Thanks again for putting in the work on this - I'll get to the next review faster :)
|
Thanks for the detailed feedback! I'll make those changes and resubmit. |
This PR adds optional station-level parental controls.
A global PIN can be configured in
main_config.json, and individual stations can opt into requiring that PIN before playback begins. When a protected station is selected, FS42 displays a simple PIN prompt instead of immediately showing the station content.Configuration
Add a global PIN in
main_config.json:Then enable parental controls on any station by adding this to any station's conf:
If
parental_controlsis omitted or set tofalse, the station behaves normally.Behavior
When a protected station is selected, FS42 displays a persistent PIN prompt before playback.
Number input from the remote is captured as PIN entry instead of channel entry. A correct PIN unlocks the station and begins normal playback. An incorrect PIN clears the entry and allows another attempt.
Channel up/down can still be used to leave the protected station without entering the PIN. Leaving and returning to the protected station requires the PIN again.
The lock screen is generated at runtime as a simple image, avoiding dependencies.
The feature uses a small runtime status file so the remote controller can determine when numeric input should be treated as PIN entry.
Known issues
This doesn't work with the web remote. The web remote doesn't send keypresses when numbers are pressed, it instead changes the channel directly. I didn't patch the web remote or change the way it worked. If you're using the web remote, you'll need to change the channel with
curl -s -X POST "http://127.0.0.1:4242/player/parental/digit/Xwhere X is the digit you want to enter