Lifecycle fixes, diagnostic sensors, resolution select and talk-back (1.2.0) - #14
Open
DevLn wants to merge 68 commits into
Open
Lifecycle fixes, diagnostic sensors, resolution select and talk-back (1.2.0)#14DevLn wants to merge 68 commits into
DevLn wants to merge 68 commits into
Conversation
- Import `get_config` from `.config_helpers`. - Update configuration handling to use schema validation. - Add debug logging for configuration details.
ensure_connected() opened a fresh P2P session per operation and tore it down immediately afterwards, racing the Close packet ahead of fire-and- forget binary commands (PTZ, lights) so they often never executed (devbis#5). The previous workaround — await asyncio.sleep(1) before closing — only made the race less likely while adding a full second of latency and a discovery+handshake to every command. Replace it with reference-counted, lock-guarded lifecycle that keeps the session open for a configurable idle window after the last operation: - connect()/close() now run under an asyncio.Lock, fixing a concurrent-open race where two callers could each start a session (one leaking). - When the refcount hits zero, teardown is deferred via an idle task; a command arriving within the window reuses the live session. The task re-checks the refcount under the lock before closing, so a reconnect during the wait can't be closed out from under. - The window is configurable via `idle_disconnect_delay` (default 5s, 0 = disconnect immediately). Verified the lifecycle (idle teardown, burst reuse, concurrent connects, reconnect-during-teardown guard) with a standalone asyncio test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
handle_async_mjpeg_stream ended with `return response` inside a `finally`, suppressing the CancelledError raised when Home Assistant tears down the stream (client navigates away). That defeated cancellation and kept the streaming coroutine — and its warm camera session — alive. Keep only the log line in `finally` and return after the `async with` exits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PPPPDevice.available was set True once and never updated, so entities always reported available even when the camera was unplugged or unreachable. Drive it from the connect path: mark available on a successful (re)connect and unavailable when device.connect() fails, and notify entities through a dispatcher signal they subscribe to in async_added_to_hass. Also roll back the connection refcount when connect() fails — ensure_connected() does not run its close() in that case, so the reference was leaking. Note: an asynchronous mid-session drop is detected lazily, on the next operation that has to reconnect (aiopppp clears its session on loss); the library exposes no device-lost callback to hook here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lamp switches/lights always started 'off' and flipped _attr_is_on before the command was sent, so a lamp already on at startup showed as off and a failed command still moved the UI. Initialize the state from the camera's reported properties (lamp/icut) and only update it after the command succeeds. Mark the entities assumed_state, since these cameras can't reliably report lamp state back. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
It was annotated Callable[[PPPPDevice], bool] but is called with (device, hass), like the switch and button descriptions. Correct the hint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The camera entity hard-coded _attr_is_streaming = True, so it always reported "streaming" regardless of reality. Surface the actual state and let it be controlled: - A: is_streaming now derives from the live session (is_connected and is_video_requested) instead of a constant. - B: subscribe to a new per-device "streaming" dispatcher signal; PPPPDevice forwards the library's on_video_state_change callback to it, so the entity refreshes whenever streaming starts or stops for any reason (including a stalled-stream drop or session teardown). - C: advertise CameraEntityFeature.ON_OFF with async_turn_on/async_turn_off mapped to start_video/stop_video. turn_on holds a connection reference so the stream persists until turn_off releases it. Requires the aiopppp on_video_state_change callback (new in the library). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surface the configured IP in the Firmware field (sw_version): the camera has no web UI (so a configuration_url "Visit" link is useless) and reports its own ipAddr as zeros, so this is the only way to show the address as plain text, like some other integrations do. Use the full device ID (e.g. PTZA-...-...) as both model and serial_number; drop the bare numeric serial and the manufacturer that added no value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Local brand images (HA 2026.3+): Home Assistant's stock generic-camera icon with a small Arial Black "PPPP" label composited into the bottom-right of the camera body -- so the glyph is pixel-crisp and exactly HA's size, not a smaller re-drawn copy. icon.png (256x256) and icon@2x.png (512x512). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
aiopppp now handles a P2pRdy/handshake timeout cleanly, so Device.connect()
raises NotConnectedError ("Device lost during connection") instead of a bare
TimeoutError. Catch it too in async_setup_entry so a camera that is briefly
flaky at startup yields ConfigEntryNotReady (auto-retry) instead of a failed
config entry.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
async_unload_entry unloaded only PLATFORMS (camera), leaving the lamp and button entities orphaned on unload/reload, and it never tore down the warm session or dropped the hass.data reference -- so the session, socket and pending idle-unload task leaked across reloads. Unload the platforms that were actually set up, then async_stop() the device and pop it from hass.data. Also stop registering a second options-update listener in async_setup_entry: PPPPDevice.async_setup already registers one that reloads the entry, so the pair caused a double reload on every options change. Guard the connect-failure cleanup against device.device not existing yet, and let _idle_unload swallow cancellation that arrives while awaiting the lock (not only during the sleep). Drop the leftover `import select` and the now-unused async_reload_entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- async_perform_reboot went straight to session.reboot(), which fails when
the session was idle-closed; route it through device.async_reboot() so it
reconnects first. Fix its docstring ("PTZ action" -> reboot).
- PTZ dropped the tilt axis when both pan and tilt were supplied (elif);
apply both independently.
- Lamp entities gated IR availability on the white-lamp "lamp" property in
all three platforms; gate each on its own property via LAMP_STATE_PROPERTY
so an IR-only or lamp-only camera exposes the right entity.
- The reboot button was gated on the unrelated "auth" login flag (hidden
when login failed); always expose it.
- Set should_poll=False on the base entity: these are dispatcher-driven /
assumed-state and implement no async_update, so polling was wasted work.
- Drop a duplicate "Getting camera image" log line.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The discovery loop ran via a detached hass.loop.create_task and was never cancelled, so it kept running with zero config entries and past shutdown. Use hass.async_create_background_task, keep a handle, and cancel it on EVENT_HOMEASSISTANT_STOP. - A fresh PPPPDiscovery was built every iteration, resetting the "already discovered" set, so each known camera re-raised a discovery flow every interval. Reuse a single instance so dedup persists. - Misconfigured/empty discovery IPs raised HomeAssistantError that the loop logged as an error every interval; return [] and warn once instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Enable the previously commented-out options flow, exposing idle_disconnect_delay as a per-entry override (falling back to the YAML global). get_idle_disconnect_delay now accepts the config entry and prefers its options; PPPPDevice reads the per-entry value. The device's existing options-update listener reloads the entry so a change takes effect immediately. Also broaden async_validate_input so any connection failure (not just a timeout) surfaces as cannot_connect instead of aborting the flow, and add the options-step translations. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Surface the library's new binary-protocol features: - ptz_preset service (goto/set) on the camera entity, wired through a new PPPPDevice.async_ptz_preset helper to session.ptz_goto_preset/set_preset. - A sensor platform with battery, signal-strength, and uptime diagnostic sensors, created only when the camera reports the value (JSON batValue/ signal, binary batLevel). Added Platform.SENSOR to the entry setup. - A "Sync time" button (binary cameras) that sets the camera clock to Home Assistant local time via the new session.set_datetime, plus the PPPPDevice.async_sync_datetime helper. - services.yaml + translations for ptz/reboot/ptz_preset, the new sensors, and the sync-time button. Depends on the aiopppp release that adds these session methods; the manifest pin is bumped separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Require the aiopppp 0.3.0 release that adds the binary-protocol features this integration now exposes (PTZ presets, sensors, time sync). Also correct iot_class from local_push to local_polling: video is pulled and lamp state is assumed, nothing is pushed. Note: aiopppp 0.3.0 must be published to PyPI for this pin to resolve on a fresh install; the local editable version already matches for end-to-end testing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- The battery sensor fed binary cameras' batLevel (millivolts) into a percentage sensor, showing readings like 4213%. Use aiopppp's derived batPercent (vendor-app thresholds; None when externally powered), with JSON cameras' batValue unchanged. - Sync-time passed the host's east-positive UTC offset as tz_seconds, but the camera stores seconds WEST of UTC -- every sync inverted the zone (UTC+3 became UTC-3). aiopppp>=0.4.0 computes the correct wire value when tz_seconds is unset, so pass only the timestamp. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New diagnostic sensors sourced from the camera status block (binary cameras): firmware version, power source (external/battery enum, from aiopppp's externalPower), SD card usage %, and timezone. All EntityCategory.DIAGNOSTIC; the noisier ones default-disabled. Also guard the existing uptime sensor against firmwares that report a negative (garbage) uptime. Translations added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
New pppp_camera.talk service plays any media/TTS source to the camera speaker. ffmpeg (already a dependency) transcodes the media to 8 kHz mono 16-bit PCM; the session encodes/frames each 120 ms chunk and paces them to real time so the camera's jitter buffer isn't flooded. Guarded to cameras whose session exposes start_talk/send_audio (binary protocol). Media is picked via the standard media selector and resolved through media_source, so TTS and the media browser both work. Also bump the aiopppp pin to 0.4.0 (the session-side audio framing and all device-tested fixes) and the integration to 1.3.0. Live listen (incoming audio in the HA UI) needs WebRTC/RTSP and is left as follow-up; talk-back is the implementable half today. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upstream's last published integration is 1.1.2 (pinning aiopppp 0.2.3), so 1.2.0 is the next minor. Device testing had bumped this twice -- to 1.2.0 and then 1.3.0 -- but neither was published, and 1.3.0 would imply a 1.2.0 release that does not exist. Collapse both bumps into one and follow the library back to 0.3.0. Also refresh two comments that still described the old ticking camera-time sensor, which the clock-offset sensor replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The poll-interval options were merged into a single vol.Optional by
mistake:
vol.Optional(CONF_STATUS_POLL_INTERVAL,
CONF_INFO_POLL_INTERVAL,
default=DEFAULT_STATUS_POLL_INTERVAL)
voluptuous' second positional argument is `msg`, so info_poll_interval
was never a key at all -- setting it in configuration.yaml failed with
"extra keys not allowed", and a bad status_poll_interval reported the
string "info_poll_interval" as its error message. Split into two
markers and document both in the module docstring.
The README still described the upstream state: JSON-only support, FTYC
video broken, audio "TBD", and none of the entities added since. Rewrite
the feature list and device table, and add sections for the entities,
the demand-driven polling model, and the services. The device table is
flagged as library-verified, since the HA side has not been run against
hardware yet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
async_process_play_media_url was called as an attribute of media_source,
where it does not exist -- it lives in media_player. Every talk action on
a media-source item died with AttributeError, surfaced in the UI as the
useless "Unknown error". media_player is already imported here for the
ATTR_MEDIA_* constants, so this adds no new coupling.
The step is required, not incidental: media_source resolves to a signed
but relative URL ("/media/local/x.mp3?authSig=..."), and ffmpeg needs an
absolute one.
Also stop sending ffmpeg's stderr to DEVNULL. A URL it could not fetch
produced no PCM, no exception and no log line -- the action reported
success and the camera stayed silent, which is the worst possible
outcome while testing. stderr is now drained concurrently (it would
otherwise block once the pipe buffer fills) and the tail is reported in
a HomeAssistantError when no audio was produced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Confirmed working on a PTZA camera from a local media-source file. The rest of the HA-side entities remain unexercised, so narrow the blanket "untested" caveat rather than dropping it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Testing in a live Home Assistant covered everything except the SD card usage sensor (no card available), so replace the "only talk-back is confirmed" caveat with what was actually verified. Add a Known issues section for the resolution being overwritten with HD at stream start, with the workaround (set it while streaming) and a pointer from Troubleshooting. Two library capabilities the integration inherits were missing from the feature list: automatic reconnection with backoff, and discovery probing with the extended search packet as well as the plain one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aiopppp no longer publishes 'uptime': the status field the vendor SDK names sysUptime turned out to be the Wi-Fi RSSI, not a duration, so the library reports it as 'dbm' instead. The uptime sensor could therefore never become supported again -- remove it and its translation. The signal sensor was in no poll group, a leftover from when that field was believed unusable. Now that it carries a real reading from the status block, put it in the status group so it actually refreshes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Closing alongside devbis/aiopppp#13, which this depends on -- a device-status field is decoded wrongly there (the SDK's There are also a few related findings still to verify on hardware before this is worth your time. Will reopen once both are fixed and retested. Sorry for the noise. |
Commit ab00654 removed it, calling "auth" an unrelated login flag and asserting that reboot works without a login. Neither was true and there was no capture behind it: "auth" records whether the USER_CHK handshake succeeded, and reboot is one of the few commands a camera refuses without one -- PTZA fw 2.2.15.93 answers -1015 USER_NO_PRIVILEGE. Only this button needs the gate. Lights, PTZ, resolution and time sync all work on an unauthenticated session, so they stay gated on property presence as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lamp entities seeded from `lamp` and `icut`, which are not trustworthy on every firmware: `lamp` is derived from the status block's function bitmap and reads 0 whenever the camera leaves that word unpopulated, and `icut` sits at 1 on PTZA no matter what the IR is doing -- so the IR entity could start out claiming "on" with nothing behind it. Add LAMP_REPORTED_PROPERTY (funcFillLight / funcIrLed), which aiopppp reports as None exactly when the camera didn't populate the word. Where a camera does report them (confirmed on FTYC) the lamp now seeds true state, claims the status poll group and drops assumed_state -- so a change made from the vendor app shows up in Home Assistant, and the UI is a real toggle instead of the two-button assumed-state control. Where it doesn't, the previous assume-our-own-writes behaviour is untouched, and no poll is claimed for a value that would be ignored. State is cached rather than read live so a just-sent command shows immediately and is corrected on the next poll, instead of flickering back to a stale reading in between. The seeding, the poll claim and the turn_on/turn_off path were duplicated between the switch and light platforms, so they move to a shared PPPPLampEntity; both platforms are now just their entity description plus the light's colour mode. PPPPBaseEntity grows a _handle_device_update hook for this: the signal it listens to fires after every poll, not only on an availability change, so subclasses that cache state need to adopt the new reading before writing. Device type: the device registry has no free-form attributes, so the rendered string goes in `model` -- which until now duplicated the DID already shown as serial_number -- and the raw halves go on an opt-in diagnostic sensor. Both render as e.g. "XR_PTZ (chip 2)", falling back to the raw number for either half, since the enums are transcribed from the vendor apps and are known to be incomplete. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
model goes back to the device id. The type moves to manufacturer, which is otherwise unset -- no real manufacturer is discoverable over PPPP -- and is rendered as "DevType/ChipType", e.g. "BK_A9/TX_817_810". Names only: a half aiopppp can't name is dropped rather than shown as a bare number, so PTZA reads "XR_PTZ" instead of "XR_PTZ (chip 2)". The numbers are not lost -- they are what the device_type sensor's attributes are for. That sensor is now keyed on the raw values rather than the rendered name, so a camera whose type has no name at all still gets one: its state is unknown while the attributes carry devType and chipType, which is exactly the camera whose numbers are worth seeing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
model is devTypeName ("XR_PTZ"), falling back to the device id when aiopppp
can't name the type. model_id is chipTypeName, with no fallback: an unnamed
chip leaves the field unset rather than showing a bare number. JSON cameras
report no chip but do report an image sensor, which keeps that slot.
manufacturer goes back to unset -- it briefly held the same information,
which is now in the two fields that mean it.
The device_type sensor renders "DevType (ChipType)", or whichever half is
named, or "Unknown" when neither is. Its attributes keep carrying all four
raw values, so an unnamed type is still diagnosable -- that is what the
sensor is for. The formatting helper moves to sensor.py now that the device
info builds its fields straight from the properties.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A literal "Unknown" is a real state value that merely looks like Home
Assistant's unknown state, so states('sensor.x') would return "Unknown"
where every other unknown sensor returns "unknown" and a template written
the usual way would silently never match. None renders identically in the
UI and carries the right semantics; the only cost is a gap in history,
which is meaningless for a value that never changes.
The attributes still carry all four raw values, so a camera whose type has
no name is diagnosable even while the state reads unknown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cameras that populate the function bitmap dropped assumed_state, which gave them a real toggle while the rest kept the two-button control -- so two cameras side by side looked like different kinds of entity. Worse than the inconsistency, it overstated what we know. These firmwares have repeatedly turned out to carry status fields that look populated but are not: PTZA parks `icut` at 1 whatever the IR is doing and leaves its whole powerSupply word at zero, and the sysUptime field was really Wi-Fi dBm. A model we haven't tested could report a lamp state that is quietly wrong in the same way, and a toggle claims a certainty we don't have. The reading itself is still used where a camera provides one -- seeding, the status poll and live correction are unchanged, so a change made from the vendor app still shows up. Only the UI affordance is uniform now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"-17,579 s" is hard to read as nearly five hours. Add a second sensor rendering the same value as "-4 h 52 m 59 s", dropping empty units so a small offset stays "12 s", and including days because a camera with a wrong date rather than a wrong clock shows up here as a huge number. A separate entity rather than formatting the existing one: an entity's state IS what Home Assistant displays, so there is no display-only formatting, and clock_offset has to stay a plain number for templates, automations and statistics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the pair added in the previous commit. The state is now the
human-readable form ("-4 h 52 m 59 s") since that is what Home Assistant
displays, and the plain number moved to an `offset_seconds` attribute for
templates and automations.
The unit and state_class had to go with it: both declare a numeric state,
and HA logs an error on every update when the state isn't one. That also
costs long-term statistics for this value, which is an acceptable trade for
a diagnostic that is read rather than graphed.
Note for anyone upgrading: automations reading this entity's state get text
now and should move to the offset_seconds attribute.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Restores the numeric state and puts the words in an offset_text attribute, which is the inverse of the previous commit -- and adds device_class=DURATION, which is what makes that work. DURATION keeps the state a plain number, so templates, automations and long-term statistics all come back, while letting Home Assistant convert the displayed unit per entity: a pathological offset can be read in hours without the integration hard-coding a unit. Seconds stays the default because a healthy clock is off by seconds, and hours would render those as "-0.0 h". Not timestamp: that renders relative to now, so a perfectly synced camera would appear to fall further behind the longer it had been since the last poll -- the apparent offset would track the poll interval rather than the camera. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The value is an int, but a convertible unit makes Home Assistant render decimals by default, so a three-second offset showed as "3.00 s". Suggest a display precision of 0; it stays overridable per entity, and HA scales it when the displayed unit is converted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Reopened alongside devbis/aiopppp#13, which this depends on. Merge blocker, stated up front: Changed since this was closed:
All of it was exercised against two cameras (PTZA and FTYC) in a live Home Assistant, not just byte-compiled. |
Contributor
Author
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.
Companion to devbis/aiopppp#13 — this exposes the new binary-protocol
capabilities in Home Assistant and fixes a set of lifecycle bugs found while
running the integration against real cameras.
Verified in a live Home Assistant against PTZA and FTYC cameras: camera entity,
lamps, buttons, diagnostic sensors, resolution select, clock sync, talk-back,
both services, discovery, and the config/options flows. The one entity I could
not verify is the SD card usage sensor — no SD card available.
As with the library PR, I'm happy to split this along the headings below.
Lifecycle and connection fixes
entry permanently.
log spam is gone.
CancelledErroris no longer swallowed in the MJPEG stream handler, so abrowser closing the stream unwinds cleanly.
Camera and existing entities
turn_on/turn_off(turn-on holds a connection reference so the stream isn't idle-closed).
of optimistically assuming the write landed.
light entity description.
New functionality
sensor (an offset rather than a ticking clock, so it doesn't spam the recorder).
pppp_camera.talkplays a media/TTS source to the cameraspeaker via ffmpeg.
demand-driven — only what live entities actually need is polled. Both intervals
are configurable globally in YAML and per-camera in the options flow.
and the clock is re-read after a sync so the sensor doesn't show a stale offset.
Bug fixes worth calling out
returned garbage).
info_poll_intervalwas accepted in the docs but never registered in the YAMLschema —
vol.Optional's second positional argument ismsg, not a secondkey, so the marker silently swallowed it and setting it in
configuration.yamlfailed with "extra keys not allowed".
async_process_play_media_urlonmedia_source; it lives inmedia_player, so every local file raisedAttributeErrorand surfaced as "Unknown error". ffmpeg's stderr was alsogoing to
DEVNULL, so a URL it couldn't fetch produced silence and asuccessful-looking action — it now drains stderr concurrently and raises
HomeAssistantErrorwith the tail when no audio was produced.Known issue (documented, not fixed)
Setting the resolution while the camera is idle gets overwritten with HD when the
stream starts, because the library re-asserts HD shortly after stream start (that
re-assert is deliberate — the cameras self-downgrade otherwise). Recorded in the
README of both repos with a proposed fix; not implemented because making the
choice sticky would apply it to every later stream, snapshots included.
Workaround: set the resolution while the stream is running.
Versioning
Numbered 1.2.0 (next minor after 1.1.2). Nothing published; the HACS release
is yours to cut.