| Message ID | 20250707171059.3259-1-nick.hollinghurst@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | e6fb24ffdb3efb66829a8b0a8ce5628e4552ee30 |
| Headers | show |
| Series |
|
| Related | show |
HI Nick Thanks for the fix. On Mon, 7 Jul 2025 at 18:11, Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> wrote: > > A previous change introduced a bug in which it reported AfStateIdle > when idle in Auto mode, when it should continue to report the most > recent AF cycle's outcome (AfStateFocused or AfStateFailed). > > Also fix the Pause method so it won't reset state to AfStateIdle > when paused in Continuous AF mode (to match documented behaviour). > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> Maybe you could add a "Fixes" tag that identifies the commit where things went wrong? With that small change: Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Tested-by: David Plowman <david.plowman@raspberrypi.com> Thanks David > --- > src/ipa/rpi/controller/rpi/af.cpp | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/src/ipa/rpi/controller/rpi/af.cpp b/src/ipa/rpi/controller/rpi/af.cpp > index eaaca4bc..26e59930 100644 > --- a/src/ipa/rpi/controller/rpi/af.cpp > +++ b/src/ipa/rpi/controller/rpi/af.cpp > @@ -810,10 +810,10 @@ void Af::prepare(Metadata *imageMetadata) > else > status.pauseState = AfPauseState::Running; > > - if (scanState_ == ScanState::Idle) > - status.state = AfState::Idle; > - else if (mode_ == AfModeAuto) > + if (mode_ == AfModeAuto && scanState_ != ScanState::Idle) > status.state = AfState::Scanning; > + else if (mode_ == AfModeManual) > + status.state = AfState::Idle; > else > status.state = reportState_; > status.lensSetting = initted_ ? std::optional<int>(cfg_.map.eval(fsmooth_)) > @@ -954,8 +954,10 @@ void Af::pause(AfAlgorithm::AfPause pause) > scanState_ = ScanState::Trigger; > } else if (pause != AfPauseResume && !pauseFlag_) { > pauseFlag_ = true; > - if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) > - goIdle(); > + if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) { > + scanState_ = ScanState::Idle; > + scanData_.clear(); > + } > } > } > } > -- > 2.39.5 >
Thanks, David On Mon, 7 Jul 2025 at 21:11, David Plowman <david.plowman@raspberrypi.com> wrote: > > HI Nick > > Thanks for the fix. > > On Mon, 7 Jul 2025 at 18:11, Nick Hollinghurst > <nick.hollinghurst@raspberrypi.com> wrote: > > > > A previous change introduced a bug in which it reported AfStateIdle > > when idle in Auto mode, when it should continue to report the most > > recent AF cycle's outcome (AfStateFocused or AfStateFailed). > > > > Also fix the Pause method so it won't reset state to AfStateIdle > > when paused in Continuous AF mode (to match documented behaviour). > > > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> > > Maybe you could add a "Fixes" tag that identifies the commit where > things went wrong? > With that small change: > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> > Tested-by: David Plowman <david.plowman@raspberrypi.com> I'll add that to a v2 shortly. Nick > > --- > > src/ipa/rpi/controller/rpi/af.cpp | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/src/ipa/rpi/controller/rpi/af.cpp b/src/ipa/rpi/controller/rpi/af.cpp > > index eaaca4bc..26e59930 100644 > > --- a/src/ipa/rpi/controller/rpi/af.cpp > > +++ b/src/ipa/rpi/controller/rpi/af.cpp > > @@ -810,10 +810,10 @@ void Af::prepare(Metadata *imageMetadata) > > else > > status.pauseState = AfPauseState::Running; > > > > - if (scanState_ == ScanState::Idle) > > - status.state = AfState::Idle; > > - else if (mode_ == AfModeAuto) > > + if (mode_ == AfModeAuto && scanState_ != ScanState::Idle) > > status.state = AfState::Scanning; > > + else if (mode_ == AfModeManual) > > + status.state = AfState::Idle; > > else > > status.state = reportState_; > > status.lensSetting = initted_ ? std::optional<int>(cfg_.map.eval(fsmooth_)) > > @@ -954,8 +954,10 @@ void Af::pause(AfAlgorithm::AfPause pause) > > scanState_ = ScanState::Trigger; > > } else if (pause != AfPauseResume && !pauseFlag_) { > > pauseFlag_ = true; > > - if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) > > - goIdle(); > > + if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) { > > + scanState_ = ScanState::Idle; > > + scanData_.clear(); > > + } > > } > > } > > } > > -- > > 2.39.5 > >
diff --git a/src/ipa/rpi/controller/rpi/af.cpp b/src/ipa/rpi/controller/rpi/af.cpp index eaaca4bc..26e59930 100644 --- a/src/ipa/rpi/controller/rpi/af.cpp +++ b/src/ipa/rpi/controller/rpi/af.cpp @@ -810,10 +810,10 @@ void Af::prepare(Metadata *imageMetadata) else status.pauseState = AfPauseState::Running; - if (scanState_ == ScanState::Idle) - status.state = AfState::Idle; - else if (mode_ == AfModeAuto) + if (mode_ == AfModeAuto && scanState_ != ScanState::Idle) status.state = AfState::Scanning; + else if (mode_ == AfModeManual) + status.state = AfState::Idle; else status.state = reportState_; status.lensSetting = initted_ ? std::optional<int>(cfg_.map.eval(fsmooth_)) @@ -954,8 +954,10 @@ void Af::pause(AfAlgorithm::AfPause pause) scanState_ = ScanState::Trigger; } else if (pause != AfPauseResume && !pauseFlag_) { pauseFlag_ = true; - if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) - goIdle(); + if (pause == AfPauseImmediate || scanState_ < ScanState::Coarse1) { + scanState_ = ScanState::Idle; + scanData_.clear(); + } } } }
A previous change introduced a bug in which it reported AfStateIdle when idle in Auto mode, when it should continue to report the most recent AF cycle's outcome (AfStateFocused or AfStateFailed). Also fix the Pause method so it won't reset state to AfStateIdle when paused in Continuous AF mode (to match documented behaviour). Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> --- src/ipa/rpi/controller/rpi/af.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)