Skip to content

Fix Hikvision default elevation configuration - #428

Merged
MateoLostanlen merged 2 commits into
pyronear:developfrom
fraztto:fix/hikvision-default-elevation
Oct 12, 2026
Merged

MateoLostanlen merged 2 commits into
pyronear:developfrom
fraztto:fix/hikvision-default-elevation

Conversation

@fraztto

@fraztto fraztto commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

The Hikvision/Linovision registry reads timeout into default_elevation_deg.
Setting elevation to -4.5° and timeout to 9 seconds therefore uses 9° for mapped
preset moves. Read elevation from its own configuration key instead.

Rebased onto current develop (48ad5f1); the PR targets develop. Per review,
trim the parameterized cases to timeout-only and elevation-only for both aliases,
plus the mapped preset movement test. Startup ISAPI and movement execution are
mocked. All five focused tests and Ruff/format checks pass on Python 3.11.

New CI runs were triggered by the updated head. GitHub currently requires maintainer
approval for these workflows before execution.

The rebased full camera suite also passes locally: 82 passed, seven skipped.

@MateoLostanlen

Copy link
Copy Markdown
Member

Thanks @fraztto for catching this, the fix is correct and the tests cover it well (9 of the 11 fail with the old line restored).

One request before merging: please rebase onto develop and retarget the PR there. In this repo PRs go to develop, and main is behind it. The same bug is still on develop (registry.py:136), so the one-line change should apply cleanly.

Small optional nit: the parametrized cases could be trimmed to timeout only and elevation only, plus the preset move test. Not blocking.

The two failing checks look unrelated: test_engine.py fails with Invalid URL 'status' (engine package, untouched here) and the Docker build hits 404s on the Debian bullseye security mirror

@fraztto
fraztto changed the base branch from main to develop October 8, 2026 17:02
@fraztto

fraztto commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, sorry about that. I've changed the base to develop. Thanks

@MateoLostanlen MateoLostanlen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix !

@MateoLostanlen
MateoLostanlen merged commit d2ba26a into pyronear:develop Oct 12, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants