Answer the owner's question: is my sensor OK, and is anything being done about it? - #35
Merged
Merged
Conversation
A tester put it plainly: the owner's question is "is my sensor OK, and can you
fix it?", and the tab answered neither part. It ended on a button that
downloaded a list of coordinates, with nothing to say whether the number in it
was bad news -- "an owner who pressed Find the bad pixels is left holding a
text file".
So the run ends on a verdict, and everything else is the working.
Whether it is a lot of stuck pixels is judged two ways, because neither is
enough alone. The arrangement decides what was found at all: a clumped set is
the picture, not the sensor, and that verdict already existed. The count is
then read as a FRACTION of the sensor, against the only published acceptance
figure this tree has -- Sony selling an IMX415 as good with up to 800 white
pixels in the dark, which on a 3864x2192 part is 9.4e-5, rounded here to a
hundredth of a percent. A fraction rather than a count so it carries to sensors
of other sizes.
It is a weak yardstick and the copy says so rather than pretending to a pass
mark. It is also strongly conditional, which matters more: the lab camera
reports 882 sites at 0.5 s and 3921 at 7 s -- dark current doing exactly what
it should -- so a verdict blind to exposure would call one healthy sensor both
fine and faulty within a minute. The exposure and the gain are in the sentence
for that reason.
The other half of the question is the camera's, so the camera is asked. Its
image profile carries the switch for its own defect corrector, and the editor
reads it through a new `sensor` provider: already correcting, or off with an
offer to turn it on behind the same hold-and-confirm a calibration uses.
Without a provider the card says such a corrector exists and that this page
cannot see it, which is the degradation Plates and Calibrate already do.
What is NOT on offer is sending this scan's coordinates to the camera, which is
what the report asked for. The static defect table is refused by the chip on
these parts and the profile carries none, so there is nowhere to put a list.
The corrector the camera runs is automatic and finds its own.
Read from the lab camera, unauthenticated, which is where the copy comes from:
GET /api/v1/isp/profile.ini
[ir_static_dpc]
DpcEnable = "1"
DpcStrength = "50, 100, 210, 235, 240, 245, 250, 255, 220, 220, 152, ..."
Note the section is named for the profile in force -- a camera on its night
profile calls it ir_static_dpc -- so the name is found rather than assumed, and
a patch is written back to the section it was read from.
The rest of what the tester listed:
- One number. "Seen in N of 5" comes from the threshold and is used by the
verdict, the sentence and the button alike; the single-frame count moved
under the details.
- Two five-capture methods on one tab, and two buttons called Capture -- their
script pressed the wrong one first. The manual scan and its own keep-and-
compare flow fold away under "Advanced -- one frame at a time", and the
wizard's button says "Take picture N of 5".
- "Where to look" threw the reading away and did not take another. It re-reads
the frame now. The check that required the old behaviour has been rewritten
rather than deleted, and says why.
- Step 1's seven lines of setup, the "Covered --" finding and a nine-line
paragraph about dark current were reprinted after every capture. They are
said once, and between captures there is a one-line reminder and the count.
- Black level, Clipping and Noise lead with a plain judgement and keep their
figures behind a disclosure. The black-level warning in particular read as a
fault with no remedy; it now says what it costs the picture and that it is
the camera's own tuning, not something this page can change.
The disclosure is the first in this tree, so it is <details> and <summary> --
they bring the keyboard and the screen reader with them, and a div with a click
handler brings neither. It is styled off .re-shead, which was already a
heading-and-rule row at fifteen call sites and is the right shape for one.
----
And the numbers it rests on, which did not agree.
A tester ran the guided defect hunt and found four numbers on the result
screen, two of which disagreed: "388 of which have turned up every time" over
a table whose IN ALL 5 row said 397. Both mean the same thing, so one of them
was wrong.
The compare panel drops the open frame's kept scan out of scanSet and puts a
synthetic entry in its place, built from diag.defects. That is right in the
manual flow, where diag.defects IS this frame's own reading. It stops being
right the moment the tally has replaced diag.defects with its own result: the
fifth capture then becomes the conclusion, offered back as evidence for itself.
The arithmetic, exactly. With c(k) the number of real captures holding site k,
the synthetic entry contributes [c(k) >= 4], so the table counted
in-all-5 = #{c = 5} + #{c = 4 and absent from capture 5}
and the nine sites in that second term had missed a capture while being
reported as having missed none. It runs the other way too: a site seen only in
the last capture came out at zero and vanished from the table altogether, which
is why the rows stopped summing to the count above them -- the fourth number.
The same fault fires in the manual flow the instant Mark is pressed, and asking
for "5 of 5" afterwards asserted that 397 sites turned up in all five when 388
had.
So the tally runs over the captures and never over its own answer: if this
frame has been kept, scanSet already holds its real reading; stand in for it
only when it has not been kept and diag is a genuine single-frame scan.
Four more found while in here:
- Mark dropped `partial` when it rebuilt fromTally, so pressing it turned the
exported header's "# complete" line from NO to "yes, as far as the scan could
see" on a click that changed no evidence -- while the warning on screen, which
reads a different flag, went on saying the opposite.
- "That capture did not arrive. The message above says why." was shown when
takeFrame returned in silence because the editor was busy, which posts no
message anywhere. That case has its own sentence now, and the step button is
disabled until there is a frame and the editor is idle -- it used to be live
before the first frame had loaded, which is how the tester reached it.
- The deviation histogram is one frame's, and after a tally it rode along on
the spread of diag under a headline about five captures.
- Changing "where to look" mid-run left scanSet holding captures read under
different gates, which the tally counted as comparable. The run owns the gate
while it is open.
The regression test needed a fixture the suite could not produce: every
existing one plants the same shared sites in every capture, so nothing is ever
seen in exactly four and the two numbers agree whatever the code does. The new
one drops the shared sites from the last capture only. Watched failing with the
fix reverted.
PR Summary by QodoAdd actionable sensor diagnosis and correct defect tallies
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Three presentation faults, all of them editor-wide rather than anything to do
with the tab they were found on.
The inspector rail is flex: 0 0 328px with no min-width, inside a row that
never wraps. At 400px that leaves the picture 71px of a 400px screen -- 43px of
it once the stage's own padding is taken -- so a phone showed a strip beside a
panel. Under 560px the two stack instead: the rail goes full width below the
picture, the stage keeps a floor of 42vh, and the rail's left border becomes a
top one. The DOM is already picture-then-rail, so nothing moves.
The tab bar is white-space: nowrap inside .re-top > * { flex: 0 0 auto }, with
no overflow declared anywhere. A fifth tab and the Capture and Download buttons
after it simply left the screen, with nothing to scroll to reach them. The bar
scrolls now.
And the tertiary text colour, #878a94, is 4.32:1 on the raised #232734 that
panels are drawn in -- under the 4.5:1 minimum, on the class that carries every
piece of explanatory prose in the editor. It passes on the rail, at 4.78:1,
which is presumably how it survived. #a2a6b2 is 6.12:1 on the panel and 6.77:1
on the rail. Measured with the WCAG formula, checked against known pairs first
(white on black 21.00, #767676 on white 4.54); two other figures were in
circulation for this and both were wrong.
Touch targets: nothing in the editor reached the 44px guideline -- 32 for a
button, 28 for a segment, 25 for the main tab bar, 16 for a reset. Worse, the
720px query REDUCED button padding and stripped their labels, making them
smaller on exactly the devices most likely to be touched. Under 560px the
interactive minimum is 44px, and that query no longer shrinks anything.
The check for this needed a second browser. The rules are media queries and a
media query asks the viewport, so shrinking a host element inside the existing
1280px run cannot reach them; the harness now launches a 400px pass that runs a
short layout-only page and reports separately. It earned itself immediately:
the first version of the touch-target rules sat ABOVE .re-seg.re-tight button
in the file and lost to it on source order at equal specificity, so the tab bar
stayed at 25px and the new check said so.
Also, re-ok and re-warn were told apart by an icon colour and a 1px border at
60% alpha -- and most call sites handed both variants the same warning
triangle, so a good answer arrived wearing one. There is a tick now.
"Diagnose" is a verb with no object. It never said what was being diagnosed, so an owner wondering whether their sensor has stuck pixels had no reason to guess they were behind it -- and the tab's whole job, after the guided run went in, is to answer exactly that question. It is "Bad pixels" now. The other labels on that bar are Develop, Calibrate, Plates and Focus: three of them name the thing you are working on. This one does too. The mode key stays `diagnose`. It is internal, it appears at around forty sites, and renaming it would be churn with nothing visible at the end of it. The checks move to the new label, which is also the first real exercise of the scrollable tab bar from the commit below -- "Bad pixels" is two characters wider than what it replaces. ---- Review of the three commits below found eight things, and the two that matter most were in the part that writes to the camera. Pressing "Keep it on" assigned null to the hold's timer handle without cancelling the timer it named. The clock ran on, and when it expired it put back a change the owner had just confirmed -- while the card, reading dpc.enabled, went on saying the correction was on. It is cleared first now, before keep() is called at all. And that hold outlived the editor. Closing the page inside it left a timer that still fired and still called revert(), changing a camera whose operator had gone. destroy() cancels it and puts the change back at once instead: the hold exists because somebody is there to keep it, so once nobody is, there is nothing left to wait for. Two in the tally, one of them mine from two commits down. "Use scanSet when the frame is kept or the reading is a tally" is right after a guided run, where keepCurrentScan has already put the frame in. In the manual flow it has not, and the reading on screen stands in for it -- so Mark threw that stand-in away, dropped the capture count by one, and moved the votes and the suggested threshold with it, on a click that changed no evidence. A tally carries the frame's own keys now. And the verdict did not know about truncation: a run where a capture filled the store has a short list, by an unknown amount in a known place, so the fraction taken from it could be read as "normal" off the very evidence that went missing. It says what it does not know instead. Three about the camera card. A profile with no correction section read as a request still in flight, and since dpcAsked blocks the retry the card said "Asking the camera..." for as long as the editor stayed open; that is an answer now, with its own sentence. The reading was never asked for twice, so a second run reported the first one's state though the setting can change between them. And the palette comment named the wrong colour -- a blanket replace of the old value rewrote the sentence describing it, leaving a comment claiming #a2a6b2 was both 4.32:1 and 6.12:1 against the same background. The eighth was a check, and it was worthless in the way its own comment now admits: it compared each tab's right edge against the bar's scrollWidth, which is the width of the whole scrollable content -- an offscreen tab is inside that by definition, so it would have passed with the bar still overflowing off the screen, which is the fault it was written for. It scrolls to each tab and checks it lands in the visible box.
openipc-ai
force-pushed
the
diagnose-ux
branch
from
September 24, 2026 19:35
4053b7c to
01326aa
Compare
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.
A UX report on the guided defect run said the tab measures well and answers
neither half of the owner's question — "is my sensor OK, and can you fix it?"
It ended by downloading
defects.txtwith no verdict and nothing applied tothe camera: "an owner who pressed Find the bad pixels is left holding a text
file." It also found four numbers on one result screen, two of which
disagreed.
Investigating it changed the design three times.
The disagreeing numbers were a real defect
The compare panel dropped the open frame's genuine kept scan and substituted
one built from
diag.defects— right in the manual flow, where that is thisframe's reading, and wrong the moment the tally had replaced it with its own
result. The fifth capture became the conclusion, offered back as evidence for
itself:
The reported 397 was 388 plus nine sites that had missed a capture, listed
as having missed none. It runs the other way too — sites seen only in the last
capture came out at zero and vanished from the table, which is why the rows
stopped summing to the count above them. The same fault fired in the manual
flow the instant Mark was pressed.
No fixture could show it: every existing one plants the same shared sites in
every capture, so nothing is ever seen in exactly four and the two numbers
agree whatever the code does. The new one drops them from the last capture
only. Reverted, it reads "line says 0, table says 12".
A coordinate list cannot be applied to these cameras
The report asked for a "Hide them on this camera" button. The static
defect-pixel table is refused by the chip on these parts and the profile
carries no static table, so there is nowhere to send a list.
But the camera is already correcting them, and can say so
Read from a lab camera over HTTP, unauthenticated:
So the verdict asks the camera instead of guessing, through a new
sensorprovider, and offers to switch the corrector on only where it is off — behind
the hold-and-confirm a calibration already uses. Without a provider it says
such a corrector exists and that this page cannot see it, the way Plates and
Calibrate degrade.
It does not claim the corrector removes them from the video, because that
could not be measured: proving it needs a full-resolution snapshot beside the
raw frame, and the lab camera cannot make one — the JPEG encoder is a buffer
block short at
isp.blkCnt: 4and has no media memory at 5 or above, besidethe H.265 stream. The card says the switch is on and what it is for, and stops.
The rest
One number everywhere, from the threshold. The manual scan and its own
five-capture flow fold under "Advanced — one frame at a time", ending the two
parallel methods and the two buttons called Capture that a tester's script
tripped on. "Where to look" re-reads the frame instead of emptying the panel.
Setup advice is said once, not reprinted after every capture. Black level,
Clipping and Noise lead with a plain judgement and keep their figures behind a
disclosure — the first in this tree, so it is
<details>/<summary>, whichbring the keyboard and the screen reader with them.
Four more defects found on the way: Mark silently flipped the exported header's
completeness claim; "the message above says why" was shown when nothing had
been posted above it; the deviation histogram sat under a five-capture headline
describing one frame; and changing the gate mid-run mixed captures read under
different gates into one tally.
Presentation
The rail was
flex: 0 0 328pxinside a row that never wraps, so at 400px thepicture got 71px of a 400px screen. Under 560px they stack. The tab bar was
nowrapinsideflex: 0 0 autowith no overflow, so a fifth tab and thebuttons after it left the screen unreachable; it scrolls now. Tertiary text was
4.32:1 on the panel colour it mostly sits on — under AA, on the class
carrying every piece of explanatory prose — and is 6.12:1 now. Touch targets
reach 44px, where the 720px query had been shrinking them.
The suite had no layout coverage at all, because media queries ask the viewport
and the existing run is one 1280px browser. There is a second 400px pass now.
It earned itself immediately: the first version of the touch rules sat above
.re-seg.re-tight buttonin the file and lost on source order, so the tab barstayed at 25px and the new check said so.
Out of scope
The sensor badge reading "HiSilicon imx335" on a Goke board is the DNG's own
camera-model tag — the editor shows what the file says. The
/api/v1/peers404is the WebUI shell.