diff --git a/ios/HouseScan/Contract/ScanContract.swift b/ios/HouseScan/Contract/ScanContract.swift index 00f3e3819..1372e2ab5 100644 --- a/ios/HouseScan/Contract/ScanContract.swift +++ b/ios/HouseScan/Contract/ScanContract.swift @@ -790,6 +790,9 @@ final class ScanViewState { /// camera can't tell open sky from an eave, so the homeowner answers. var overheadQuestion = false var upload: UploadState = .idle + /// How many views the finished check's answer will still ask for without a tap, counting a + /// server request on screen: what "One more view to finish" and "2 more views to finish" promise. + var followUps = 0 var result: ResultPresentation? /// The spot check of the result's spot: the question during `.spotConfirm`, and the answer /// that stands for the spot on the result. @@ -864,8 +867,13 @@ protocol ScanActions: AnyObject { func finishWalk() /// Features confirmed; the engine runs the gap check, then uploads. func confirmFeatures() - /// "I can't get there": the gap is recorded for installer review. + /// "I can't get there": the gap is recorded for installer review. On a request the finished + /// check sent back, the check's next request follows; the result shows once none is left. func skipGap() + /// "Show my result" on a request the finished check sent back: the view is recorded for + /// installer review like a skipped one, and after one more upload the result shows instead + /// of the check's next request. + func showResultNow() /// "I can't get to this part of the wall", during the walk: the cells the guidance is asking /// for become `.skipped` and the guidance moves on to the next task. While the walk asks to /// walk a side, that side's end goes where `ScanViewState.endPreview` shows, as an unexplored diff --git a/ios/HouseScan/Runtime/Autopilot.swift b/ios/HouseScan/Runtime/Autopilot.swift index 02590edb4..20501b086 100644 --- a/ios/HouseScan/Runtime/Autopilot.swift +++ b/ios/HouseScan/Runtime/Autopilot.swift @@ -434,18 +434,46 @@ final class Autopilot { /// for it; a replay shows only what it recorded, so when they don't settle the request the /// autopilot says "I can't get there", as a homeowner would. An overhead request is answered /// like the one before the upload. + /// With `-autopilotShowResult`, "Show my result" on the first one instead. private func driveServerRequest(_ replay: ReplayPlayer) async { guard let request = engine.state.gap else { return } log("server request \(request.id) over \(format(request.span))") - if request.reason == .overhead { - await answerOverheadGap() + let skippedBefore = engine.skippedGaps.count + let action: String + if engine.options.autopilotShowResult { + await pause(hold) + await engine.waitForGate(.gapRequest) + engine.showResultNow() + log("server request \(request.id): show my result") + action = "showResult" } else { - await playGapFrames(replay) + if request.reason == .overhead { + await answerOverheadGap() + } else { + await playGapFrames(replay) + } + action = engine.skippedGaps.count > skippedBefore ? "skipped" : "answered" } + recordServerRequestForTest(id: request.id, action: action) // A settled request stays on screen for a moment before the next upload. _ = await waitUntil(timeout: 10) { self.engine.state.phase != .gapRequest || self.engine.state.gap?.id != request.id } } + /// The requests the server's answers raised, in order, with how each ended ("answered", + /// "skipped" or "showResult"): with `-autopilotGate`, written to the gate folder as + /// `server-requests.json` after each, for the UI test to check the follow-up loop (issue #39). + private var serverRequestsForTest: [[String: Any]] = [] + + private func recordServerRequestForTest(id: Int, action: String) { + serverRequestsForTest.append(["id": id, "action": action]) + guard let gate = engine.options.autopilotGate else { return } + do { + try JSONSerialization.data(withJSONObject: serverRequestsForTest).write(to: gate.appending(path: "server-requests.json")) + } catch { + log("could not write server-requests.json to the gate folder: \(error)") + } + } + /// Waits while the engine plays the replay's frames for the current request. A replay shows /// only what it recorded, so when they don't settle the request the autopilot says "I can't /// get there", as a homeowner would. diff --git a/ios/HouseScan/Runtime/LaunchOptions.swift b/ios/HouseScan/Runtime/LaunchOptions.swift index e5deeb9c6..b7da29b74 100644 --- a/ios/HouseScan/Runtime/LaunchOptions.swift +++ b/ios/HouseScan/Runtime/LaunchOptions.swift @@ -20,6 +20,8 @@ import OSLog /// marking the ends (`Autopilot.endWalkByCantGetThere`). /// - `-autopilotSomethingThere`: the autopilot answers the first spot check "Something's there" /// instead of "It's clear", so the scan is checked again without that area. +/// - `-autopilotShowResult`: the autopilot taps "Show my result" on the first request the +/// server's answer raises, so no further request is raised (issue #39). struct LaunchOptions: Equatable { var replayFolder: URL? var autopilot = false @@ -29,6 +31,7 @@ struct LaunchOptions: Equatable { var autopilotGate: URL? var autopilotCantGetThere = false var autopilotSomethingThere = false + var autopilotShowResult = false init( arguments: [String] = ProcessInfo.processInfo.arguments, @@ -44,6 +47,7 @@ struct LaunchOptions: Equatable { autopilot = arguments.contains("-autopilot") autopilotCantGetThere = arguments.contains("-autopilotCantGetThere") autopilotSomethingThere = arguments.contains("-autopilotSomethingThere") + autopilotShowResult = arguments.contains("-autopilotShowResult") serverURL = (value(after: "-serverURL") ?? defaultServerURL).flatMap(Self.serverURL) sampleResult = arguments.contains("-sampleResult") if let gate = value(after: "-autopilotGate") { autopilotGate = URL(fileURLWithPath: gate, isDirectory: true) } diff --git a/ios/HouseScan/Runtime/ScanEngine+Actions.swift b/ios/HouseScan/Runtime/ScanEngine+Actions.swift index 3149a6303..c567d5ce9 100644 --- a/ios/HouseScan/Runtime/ScanEngine+Actions.swift +++ b/ios/HouseScan/Runtime/ScanEngine+Actions.swift @@ -355,6 +355,11 @@ extension ScanEngine: ScanActions { skipCurrentGap() } + func showResultNow() { + guard state.phase == .gapRequest else { return } + stopGapRequests() + } + func cannotAccessArea() { switch state.phase { case .gapRequest: diff --git a/ios/HouseScan/Runtime/ScanEngine+Export.swift b/ios/HouseScan/Runtime/ScanEngine+Export.swift index def58ab09..a88a3799f 100644 --- a/ios/HouseScan/Runtime/ScanEngine+Export.swift +++ b/ios/HouseScan/Runtime/ScanEngine+Export.swift @@ -193,7 +193,9 @@ extension ScanEngine { let plans = result.missingEvidence.map { item in gapPlanner.plan(for: item, leftEnd: coverage?.leftEnd, rightEnd: coverage?.rightEnd, limitEnds: coverage?.limitEnds ?? []) } - let settles = plans.map { plan in plan.map { !skippedGaps.contains($0) && captureCanSettle($0) } ?? false } + let settles = plans.map { plan in + plan.map { plan in !skippedGaps.contains { plan.asksForSameView(as: $0) } && captureCanSettle(plan) } ?? false + } return Self.presentation(of: result, isSample: isSample, wall: sceneWall) { item in result.missingEvidence.firstIndex(of: item).map { settles[$0] } ?? false } diff --git a/ios/HouseScan/Runtime/ScanEngine.swift b/ios/HouseScan/Runtime/ScanEngine.swift index 60013075d..15523289e 100644 --- a/ios/HouseScan/Runtime/ScanEngine.swift +++ b/ios/HouseScan/Runtime/ScanEngine.swift @@ -101,11 +101,12 @@ final class ScanEngine { /// The end the past_end request cleared: where it was, its kind and when it was marked, put /// back or moved on when the request ends without it marked again (`settleClearedEnd`). private var clearedEnd: (s: Float, kind: EndKind, t: Double?)? - /// Server requests raised without a tap since the review was confirmed (`nextAutomaticGap`), - /// oldest first. Each is raised once; the result still offers it as a capture. + /// Server requests raised without a tap since the review was confirmed (`automaticGapQueue`), + /// oldest first. Each is raised once, whether its view was taken or the homeowner couldn't + /// get there; the result still offers it as a capture. private var automaticGaps: [GapPlan] = [] - /// Set when the homeowner says they can't get to a server request's view: after the upload - /// that follows, the result shows instead of the next request. + /// Set when the homeowner taps "Show my result" on a server request (`stopGapRequests`): + /// after the upload that follows, the result shows instead of the next request. private var automaticGapsStopped = false /// At most this many server requests are raised without a tap per confirmed review, so a /// server that keeps finding new gaps can't hold the homeowner in the loop. A guess, not @@ -1394,15 +1395,14 @@ final class ScanEngine { RuntimeLog.engine.info("overhead: kept a view reaching \(reach.map(\.out).min() ?? 0) m over s=\(reach.first?.span.lowerBound ?? 0)...\(reach.last?.span.upperBound ?? 0)") } - /// Ends the current request without the view it asked for ("I can't get there", or something - /// overhead): recorded for installer review, then on to the upload. "I can't get there" on a - /// server request also ends the requests raised without a tap: after that upload the result - /// shows. Something overhead is an answer, not a refusal, so the loop goes on. + /// Ends the current request without the view it asked for ("I can't get there", something + /// overhead, or "Show my result"): recorded for installer review, then on to the upload. The + /// answer that follows raises the next item it lists, never this one again + /// (`automaticGapQueue`); only "Show my result" (`stopGapRequests`) ends the requests. func skipCurrentGap(because reason: String = "the homeowner can't get there", refused: Bool = true) { guard let plan = gapPlan else { return } // Something overhead is an answer: the request goes to review without its view. resolveGuidance(refused ? .cannotReach : .skipped) - if refused, state.gap?.origin == .server { automaticGapsStopped = true } // Only a cell request marks cells: skipping a deeper, walked or overhead view says // nothing about the band the strip draws. if plan.need == .cells { coverage?.markSkipped(plan.band, plan.span) } @@ -1412,6 +1412,15 @@ final class ScanEngine { afterGapResolved() } + /// "Show my result" on a server request: this view goes to installer review like one the + /// homeowner can't get to, and after the upload that follows the result shows instead of + /// the answer's next request. + func stopGapRequests() { + guard gapPlan != nil, state.gap?.origin == .server else { return } + automaticGapsStopped = true + skipCurrentGap(because: "the homeowner asked for the result", refused: false) + } + // MARK: Upload func startUpload() { @@ -1512,10 +1521,11 @@ final class ScanEngine { guard let result = placement else { return } writeScanStamp(answer: result) state.result = presentation(of: result, isSample: resultClient.isSample) + state.followUps = automaticGapQueue(result).count state.upload = .done await waitForGate(.uploading) guard scan == generation else { return } - if let next = nextAutomaticGap(result) { + if let next = automaticGapQueue(result).first { // The upload screen says "One more view to finish" once the answer is in // (`state.result` set, upload `.done`, both kept through the request): long // enough to read before the camera takes over. @@ -1531,24 +1541,31 @@ final class ScanEngine { presentAnswer() } - /// The first item of the answer's missing evidence a capture can settle (the result's - /// "capturable"), not skipped and not yet raised in this pass; nil once the homeowner said - /// they can't get to one, or after `maxAutomaticGaps` requests. - private func nextAutomaticGap(_ result: PlacementResult) -> (item: PlacementMissingEvidence, plan: GapPlan)? { + /// The answer's items this pass will still raise without a tap, in order, with their + /// requests: those a capture can settle (the result's "capturable"), not skipped and not yet + /// raised in this pass (`GapPlanner.serverRequests`), up to `maxAutomaticGaps` requests. + /// Empty once the homeowner tapped "Show my result". `asking`, a request about to be raised, + /// counts as raised. + private func automaticGapQueue(_ result: PlacementResult, asking: GapPlan? = nil) -> [(item: PlacementMissingEvidence, plan: GapPlan)] { // A request raised while the phone has lost its place could only time out: show the result. - guard !automaticGapsStopped, automaticGaps.count < Self.maxAutomaticGaps, !state.tracking.hasLostItsPlace, - sourceState.mayCapture, let map = coverage else { return nil } - for item in result.missingEvidence { - guard let plan = gapPlanner.plan(for: item, leftEnd: map.leftEnd, rightEnd: map.rightEnd, limitEnds: map.limitEnds), - !skippedGaps.contains(plan), !automaticGaps.contains(plan), captureCanSettle(plan) else { continue } - return (item, plan) + guard !automaticGapsStopped, !state.tracking.hasLostItsPlace, sourceState.mayCapture, let map = coverage else { return [] } + let asked = automaticGaps + (asking.map { [$0] } ?? []) + // A new view cannot settle an area the homeowner has already said is obstructed. + // Filter before the request limit so refused areas do not consume the remaining slots. + let capturable = result.missingEvidence.filter { item in + gapPlanner.plan(for: item, leftEnd: map.leftEnd, rightEnd: map.rightEnd, limitEnds: map.limitEnds) + .map { captureCanSettle($0) } ?? false } - return nil + return gapPlanner.serverRequests( + in: capturable, leftEnd: map.leftEnd, rightEnd: map.rightEnd, limitEnds: map.limitEnds, + asked: asked, skipped: skippedGaps, limit: Self.maxAutomaticGaps - automaticGaps.count) } /// Starts the capture for an item of the server's missing evidence, whether tapped on the /// result or raised after an upload. func beginServerGap(_ item: PlacementMissingEvidence, plan: GapPlan) { + // Counted before a past_end request clears its end, which would change the other requests. + state.followUps = 1 + (placement.map { automaticGapQueue($0, asking: plan).count } ?? 0) var pastEnd: WallSide? if item.kind == .pastEnd, let side = item.side { // The walk has to go past the end it stopped at; that end is no longer a limit. It diff --git a/ios/HouseScan/UI/Preview/DemoEngine.swift b/ios/HouseScan/UI/Preview/DemoEngine.swift index 367e2a1cc..3aa302be7 100644 --- a/ios/HouseScan/UI/Preview/DemoEngine.swift +++ b/ios/HouseScan/UI/Preview/DemoEngine.swift @@ -313,6 +313,8 @@ final class DemoEngine: ScanActions { state.target = DemoScene.wall.world(s: 1.7, height: 0, out: 0.5) state.path = DemoScene.path(toward: 1.7) if let serverItem { + // The demo's check asks for one view at most. + state.followUps = 1 state.gap?.origin = .server state.gap?.reason = .server(detail: serverItem.text) run { engine in await engine.gapScript(span: span) } @@ -359,6 +361,7 @@ final class DemoEngine: ScanActions { state.path = [] state.target = nil state.upload = .packaging + state.followUps = 0 run { engine in await engine.uploadScript() } } @@ -507,6 +510,7 @@ final class DemoEngine: ScanActions { if !followedUp, let item = sample.missing.first(where: \.capturable) { state.shareableScan = Self.demoScan state.result = sample + state.followUps = 1 state.upload = .done followedUp = true guard await pause(1.6) else { return } @@ -549,6 +553,7 @@ final class DemoEngine: ScanActions { script?.cancel() state.shareableScan = Self.demoScan state.result = Self.reviewSample + state.followUps = 1 state.upload = .done followedUp = true if phase == .gapRequest { @@ -838,6 +843,11 @@ final class DemoEngine: ScanActions { enterUpload() } + /// The demo's check asks for one view at most, so stopping early is skipping it. + func showResultNow() { + skipGap() + } + func cannotAccessArea() { if case .seeBehind(let s) = state.guidance { obstructions = obstructions.map { (span: $0.span, skipped: $0.skipped || $0.span.contains(s)) } diff --git a/ios/HouseScan/UI/Screens/GapRequestScreen.swift b/ios/HouseScan/UI/Screens/GapRequestScreen.swift index eaf00f2e4..1c04c212e 100644 --- a/ios/HouseScan/UI/Screens/GapRequestScreen.swift +++ b/ios/HouseScan/UI/Screens/GapRequestScreen.swift @@ -6,13 +6,15 @@ import SwiftUI /// /// A request the finished check sent back (the scan came here from the upload, not from the /// review) carries "One more view to finish" above the instruction, the words the upload screen -/// just said, and its way out says it leads to the result. +/// just said. "I can't get there" skips only this view and goes on to the check's next one; +/// "Show my result", on every such request, stops asking and leads to the result. struct GapRequestScreen: View { let state: ScanViewState let actions: any ScanActions @State private var cameraSize: CGSize = .zero @Environment(\.accessibilityReduceMotion) private var reduceMotion + @Environment(\.dynamicTypeSize) private var typeSize var body: some View { ZStack { @@ -30,9 +32,7 @@ struct GapRequestScreen: View { reply: state.gap?.isSatisfied == true || asking ? nil : InstructionCard.Reply( title: "I can't get there", identifier: "action.skipGap", - hint: followUps > 0 - ? "Skips this view and shows your result. An installer will look at this part instead." - : "Skips this view. An installer will look at this part instead.", + hint: skipHint, perform: { actions.skipGap() } ), eyebrow: followUps > 0 && state.gap?.isSatisfied != true ? ScanCopy.followUp(remaining: followUps) : nil, @@ -46,6 +46,9 @@ struct GapRequestScreen: View { OverheadAnswers(actions: actions) .transition(.opacity) } + if !typeSize.isAccessibilitySize { + showResult + } if let gap = state.gap { GapProgress(gap: gap) } @@ -59,6 +62,9 @@ struct GapRequestScreen: View { depthChecked: state.depthAvailable ) } + if typeSize.isAccessibilitySize { + showResult + } } .animation(reduceMotion ? .easeOut(duration: 0.15) : Motion.settle, value: asking) } @@ -66,6 +72,32 @@ struct GapRequestScreen: View { .animation(reduceMotion ? .easeOut(duration: 0.15) : Motion.pin, value: state.gap?.isSatisfied) } + /// "Show my result", on every request the check sent back, even with one view left: the + /// upload after "I can't get there" can bring a new request in the next answer, and this is + /// the one way to stop asking (issue #39). + /// + /// It sits above the progress, within thumb reach, except at the accessibility text sizes. + /// There the card reaches the bottom edge, with its "I can't get there" reply cut off by it. + /// With "Show my result" directly under that reply, the audit failed the reply's contrast at + /// AX5 (CI run 36276179058); with the progress under it, as before, it passed. So at those + /// sizes it goes below the tape, the last thing on the screen, reached by scrolling like + /// everything else below the card at that size. + @ViewBuilder private var showResult: some View { + if followUps > 0, state.gap?.isSatisfied != true, !asking { + Button { + actions.showResultNow() + } label: { + Label("Show my result", systemImage: "checkmark.circle") + } + .buttonStyle(.secondary) + .accessibilityHint("Stops asking for views and shows your result. An installer will look at the parts you skip.") + .accessibilityIdentifier("action.showResult") + // Appears and goes at once, like the card's reply: a fading button is faint text for + // those moments, which the contrast audit fails (as `InstructionCard` found). + .transition(.identity) + } + } + /// The overhead question is up: it replaces the instruction, coaching and "I can't get there" /// until answered, as on the walk. private var asking: Bool { state.overheadQuestion && state.gap?.isSatisfied != true } @@ -73,8 +105,19 @@ struct GapRequestScreen: View { /// Views the finished check still wants, counting this one, when this request is one of /// them: a server request while the check's answer is in. Zero otherwise. private var followUps: Int { - guard state.gap?.origin == .server, let result = state.result else { return 0 } - return max(1, result.missing.filter(\.capturable).count) + guard state.gap?.origin == .server, state.result != nil else { return 0 } + return max(1, state.followUps) + } + + /// What "I can't get there" leads to: the check's next view while it wants more than this + /// one. With one left the scan is sent again, and that answer can still ask for a new view, + /// so the hint doesn't promise the result; "Show my result" does. + private var skipHint: String { + switch followUps { + case 0: "Skips this view. An installer will look at this part instead." + case 1: "Skips this view and checks your scan again. An installer will look at this part instead." + default: "Skips this view and goes on to the next one. An installer will look at this part instead." + } } private var instruction: Instruction { diff --git a/ios/HouseScan/UI/Screens/UploadingScreen.swift b/ios/HouseScan/UI/Screens/UploadingScreen.swift index 3accd333e..a4170835d 100644 --- a/ios/HouseScan/UI/Screens/UploadingScreen.swift +++ b/ios/HouseScan/UI/Screens/UploadingScreen.swift @@ -16,8 +16,7 @@ struct UploadingScreen: View { /// Views the finished check wants from the camera before the result. private var followUps: Int { - guard state.upload == .done else { return 0 } - return state.result?.missing.filter(\.capturable).count ?? 0 + state.upload == .done ? state.followUps : 0 } var body: some View { diff --git a/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GapPlanner.swift b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GapPlanner.swift index d9ecc5237..d57a0cda3 100644 --- a/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GapPlanner.swift +++ b/ios/HouseScanKit/Sources/HouseScanKit/Guidance/GapPlanner.swift @@ -115,7 +115,7 @@ public struct GapPlanner: Sendable { /// It asks for nothing above the walking band. The server's answer to the first upload names /// the wall above it where a check needs that, over the stretch the check reads and to the /// height it needs (a wall request with `out_ft`, `GapPlan.Need.wallUp`), and the engine - /// raises it at once as the next view (`ScanEngine.nextAutomaticGap`). A request from here + /// raises it at once as the next view (`ScanEngine.automaticGapQueue`). A request from here /// would have to guess both the stretch and the height (7.5 ft where the public rules need /// just over 6.5), and when the guess missed the spot the server chose, the homeowner would /// be asked for the wall twice. @@ -342,4 +342,79 @@ extension GapPlanner { public func endAfterPastEnd(_ plan: GapPlan, side: WalkSide, clearedAt old: Float) -> Float { side == .left ? min(plan.span.lowerBound, old) : max(plan.span.upperBound, old) } + + /// The items of the server's missing evidence still to ask for without a tap, in order, with + /// their requests, at most `limit` of them: each one a capture can settle whose request asks + /// for no view already raised in this pass (`asked`), skipped, or earlier in the list + /// (`GapPlan.asksForSameView(as:)`). A request the homeowner couldn't get to is in both, so + /// the answer that follows moves on to the next item rather than raising it again (issue #39). + public func serverRequests( + in missing: [PlacementMissingEvidence], leftEnd: Float?, rightEnd: Float?, limitEnds: Set, + asked: [GapPlan], skipped: [GapPlan], limit: Int + ) -> [(item: PlacementMissingEvidence, plan: GapPlan)] { + var requests: [(item: PlacementMissingEvidence, plan: GapPlan)] = [] + for item in missing where requests.count < limit { + guard let plan = self.plan(for: item, leftEnd: leftEnd, rightEnd: rightEnd, limitEnds: limitEnds) else { continue } + let seen = asked + skipped + requests.map { $0.plan } + guard !seen.contains(where: { plan.asksForSameView(as: $0) }) else { continue } + requests.append((item, plan)) + } + return requests + } + + /// The first of `serverRequests`: the next item to ask for without a tap. Nil when none is left. + public func nextServerRequest( + in missing: [PlacementMissingEvidence], leftEnd: Float?, rightEnd: Float?, limitEnds: Set, + asked: [GapPlan], skipped: [GapPlan] + ) -> (item: PlacementMissingEvidence, plan: GapPlan)? { + serverRequests(in: missing, leftEnd: leftEnd, rightEnd: rightEnd, limitEnds: limitEnds, asked: asked, skipped: skipped, limit: 1).first + } +} + +extension GapPlan { + /// How far a request's span or reach may move and still ask for the same view: 0.1 ft + /// (0.03 m), meters. Numerical jitter from the server working the request out again is a few + /// hundredths of a foot; a guess, not measured. This applies to the span only: a reach gets + /// no allowance beyond float noise (`reachNoise`). + public static let sameViewTolerance: Float = 0.03048 + + /// Float noise only, meters. A reach is compared as strictly as the server compares it: + /// `GapPlanner.progress` needs the requested out_ft itself, so a view out to 6.50 ft does not + /// meet a request out to 6.59 ft. + static let reachNoise: Float = 1e-5 + + /// Whether this request asks for no view beyond `other`'s, allowing for the server working it + /// out again from the next upload: the same band and kind of need, a reach no farther than + /// the other's, and a span inside the other's give or take `sameViewTolerance`. After "I can't + /// get there" the skipped stretch goes to review and the spot or its error margins can move, + /// so the same view comes back as, say, 2.41...7.9 ft instead of 2.4...7.9 ft. A request that + /// asks for more is a new view, even when it mostly overlaps: a span of 2...9 ft after + /// 2...8 ft, or ground out to 6.59 ft after 6.50 ft (issue #39). A met past_end request moves + /// its end on 2 m, so the next one asks for new ground and is a different view. + public func asksForSameView(as other: GapPlan) -> Bool { + guard band == other.band, need.asksNoMore(than: other.need, within: Self.reachNoise) else { return false } + // The server's own numbers when both have them: what `progress` compares against. + if let reach = requestedOutFt, let otherReach = other.requestedOutFt, reach > otherReach { return false } + let tolerance = Self.sameViewTolerance + return span.lowerBound >= other.span.lowerBound - tolerance && span.upperBound <= other.span.upperBound + tolerance + } +} + +extension GapPlan.Need { + /// The same kind of need, with any reach no more than `tolerance` meters past the other's. An + /// overhead need without a height (any recorded view) asks no more than one with a height. + func asksNoMore(than other: GapPlan.Need, within tolerance: Float) -> Bool { + switch (self, other) { + case (.cells, .cells): + return true + case let (.groundOut(a), .groundOut(b)), let (.walkOut(a), .walkOut(b)), let (.wallUp(a), .wallUp(b)): + return a <= b + tolerance + case let (.overhead(a), .overhead(b)): + guard let a else { return true } + guard let b else { return false } + return a <= b + tolerance + default: + return false + } + } } diff --git a/ios/HouseScanKit/Tests/HouseScanKitTests/ServerGapTests.swift b/ios/HouseScanKit/Tests/HouseScanKitTests/ServerGapTests.swift index 01a65c1c3..ff7397f9d 100644 --- a/ios/HouseScanKit/Tests/HouseScanKitTests/ServerGapTests.swift +++ b/ios/HouseScanKit/Tests/HouseScanKitTests/ServerGapTests.swift @@ -245,6 +245,146 @@ import Testing #expect(right.span == 4...6) } + /// Issue #39: the answer asks for two views and the homeowner can't get to the first. The + /// next answer, which still lists it, moves on to the second instead of raising the first + /// again or stopping; once both were raised, none is left and the result shows. The skipped + /// past_end request gets its end back (`ScanEngine.settleClearedEnd`), so it comes back as + /// the same request; a met one moves its end on, and the next asks for new ground. + @Test func eachServerItemIsAskedForOnceAndARefusalMovesOnToTheNext() throws { + let planner = GapPlanner() + let pastEnd = try item(#"{"kind":"past_end","side":"left","message":"Walk past the left end."}"#) + let ground = try item(#"{"kind":"band","band":"ground","span_ft":[3.0,5.5],"message":"Film the ground."}"#) + let requests = planner.serverRequests(in: [pastEnd, ground], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5) + #expect(requests.map { $0.item } == [pastEnd, ground]) + let first = try #require(requests.first) + #expect(first.plan.span == -5 ... -3) + + // "I can't get there": raised and skipped, with the end it cleared put back. + let reworded = try item(#"{"kind":"past_end","side":"left","message":"Keep walking past the left end."}"#) + let second = try #require(planner.nextServerRequest( + in: [reworded, ground], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [first.plan], skipped: [first.plan])) + #expect(second.item == ground) + + #expect(planner.nextServerRequest( + in: [reworded, ground], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [first.plan, second.plan], skipped: [first.plan])?.item == nil) + // A right past-end is a different view. + let right = try item(#"{"kind":"past_end","side":"right","message":"m"}"#) + #expect(planner.nextServerRequest( + in: [reworded, ground, right], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [first.plan, second.plan], skipped: [])?.item == right) + // Met, the left end moved on to -5 m: the next past_end request is the 2 m after it. + #expect(planner.nextServerRequest( + in: [reworded], leftEnd: -5, rightEnd: 4, limitEnds: [], asked: [first.plan], skipped: [])?.plan.span == -7 ... -5) + } + + /// The next answer works a refused view out again from the new scene, so it can come back with + /// its span moved by a few hundredths of a foot, or a reach no farther out. It is still the + /// view the homeowner said they can't get to, and isn't raised again; another stretch of the + /// band is a new view, and so is any reach farther out, which the refused view couldn't meet. + @Test func aRefusedViewWithSlightlyDifferentNumbersIsNotRaisedAgain() throws { + let planner = GapPlanner() + let refused = try item(#"{"kind":"band","band":"ground","span_ft":[2.4,7.9],"out_ft":4.833334,"message":"m"}"#) + let plan = try #require(planner.plan(for: refused, leftEnd: -3, rightEnd: 4)) + let moved = try item(#"{"kind":"band","band":"ground","span_ft":[2.41,7.9],"out_ft":4.833334,"message":"m"}"#) + #expect(planner.nextServerRequest(in: [moved], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [plan], skipped: [plan])?.item == nil) + let nearer = try item(#"{"kind":"band","band":"ground","span_ft":[2.41,7.93],"out_ft":4.8,"message":"m"}"#) + #expect(planner.nextServerRequest(in: [nearer], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [plan], skipped: [plan])?.item == nil) + let fartherOut = try item(#"{"kind":"band","band":"ground","span_ft":[2.41,7.9],"out_ft":4.9,"message":"m"}"#) + #expect(planner.nextServerRequest( + in: [fartherOut], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [plan], skipped: [plan])?.item == fartherOut) + let elsewhere = try item(#"{"kind":"band","band":"ground","span_ft":[-7.9,-2.4],"out_ft":4.833334,"message":"m"}"#) + #expect(planner.nextServerRequest( + in: [moved, elsewhere], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [plan], skipped: [plan])?.item == elsewhere) + } + + /// Requests raised without a tap keep the past-end guard (issue #35): ground past a marked + /// unexplored end isn't raised, ground past a limit end is. + @Test func theAutomaticRequestsKeepThePastEndGuard() throws { + let planner = GapPlanner() + let ground = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,4.0],"message":"m"}"#) + #expect(planner.nextServerRequest(in: [ground], leftEnd: -1, rightEnd: 1, limitEnds: [], asked: [], skipped: [])?.item == nil) + #expect(planner.nextServerRequest(in: [ground], leftEnd: -1, rightEnd: 1, limitEnds: [.right], asked: [], skipped: [])?.item != nil) + } + + /// "N more views to finish" counts what the answer will raise: two items asking for the same + /// view count once, and no more than the requests left in the pass. + @Test func theViewsLeftCountEachViewOnceUpToTheLimit() throws { + let planner = GapPlanner() + let ground = try item(#"{"kind":"band","band":"ground","span_ft":[3.0,5.5],"message":"Film the ground."}"#) + let sameView = try item(#"{"kind":"band","band":"ground","span_ft":[3.0,5.5],"message":"Another check wants it too."}"#) + let pastEnd = try item(#"{"kind":"past_end","side":"left","message":"m"}"#) + let all = [ground, sameView, pastEnd] + #expect(planner.serverRequests(in: all, leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [ground, pastEnd]) + #expect(planner.serverRequests(in: all, leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 1).count == 1) + #expect(planner.serverRequests(in: all, leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 0).isEmpty) + } + + /// A request that asks for more than one already raised is a new view, even when it mostly + /// overlaps: a longer stretch (2 to 9 ft after 2 to 8 ft, 6/7 of it already asked for) or + /// ground farther out (5.10 ft after 4.83 ft, 0.08 m more). Only span jitter within 0.1 ft, + /// with a reach no farther out, is the same view. This holds for earlier requests, skipped + /// ones and items in the same answer. + @Test func aRequestAskingForMoreIsANewView() throws { + let planner = GapPlanner() + let first = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,8.0],"out_ft":4.83,"message":"m"}"#) + let longer = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,9.0],"out_ft":4.83,"message":"m"}"#) + let farther = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,8.0],"out_ft":5.10,"message":"m"}"#) + let jitter = try item(#"{"kind":"band","band":"ground","span_ft":[1.95,8.05],"out_ft":4.8,"message":"m"}"#) + let asked = try #require(planner.plan(for: first, leftEnd: -3, rightEnd: 4)) + + // After the first was raised, or skipped. + for (raised, skipped) in [([asked], [GapPlan]()), ([asked], [asked])] { + let next = planner.serverRequests( + in: [first, jitter, longer, farther], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: raised, skipped: skipped, limit: 5) + #expect(next.map { $0.item } == [longer, farther]) + } + // Two items in one answer: each asks for more than the one before it. + #expect(planner.serverRequests( + in: [first, longer], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [first, longer]) + #expect(planner.serverRequests( + in: [first, farther], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [first, farther]) + // Asking for less than an earlier item is no new view. + #expect(planner.serverRequests( + in: [longer, first, jitter], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [longer]) + #expect(planner.serverRequests( + in: [farther, first], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [farther]) + } + + /// A reach only a little farther out is still more than was asked: ground out to 6.59 ft after + /// the same span out to 6.50 ft. `progress` needs the requested out_ft itself, so a view out + /// to 6.50 ft leaves it unmet, and it must be raised (Sam's review of e576190). A reach equal + /// to the earlier one is the same view. + @Test func aSlightlyFartherReachIsANewView() throws { + let planner = GapPlanner() + let first = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,8.0],"out_ft":6.50,"message":"m"}"#) + let farther = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,8.0],"out_ft":6.59,"message":"m"}"#) + let same = try item(#"{"kind":"band","band":"ground","span_ft":[2.0,8.0],"out_ft":6.50,"message":"again"}"#) + let asked = try #require(planner.plan(for: first, leftEnd: -3, rightEnd: 4)) + let fartherPlan = try #require(planner.plan(for: farther, leftEnd: -3, rightEnd: 4)) + #expect(!fartherPlan.asksForSameView(as: asked)) + #expect(asked.asksForSameView(as: fartherPlan)) + + // After the first was raised, or raised and skipped. + for (raised, skipped) in [([asked], [GapPlan]()), ([asked], [asked])] { + let next = planner.serverRequests( + in: [same, farther], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: raised, skipped: skipped, limit: 5) + #expect(next.map { $0.item } == [farther]) + } + // Two items in one answer. + #expect(planner.serverRequests( + in: [first, farther], leftEnd: -3, rightEnd: 4, limitEnds: [], asked: [], skipped: [], limit: 5).map { $0.item } == [first, farther]) + } + + /// An overhead request without a height takes any tilt-up view, so it asks for no more than + /// one with a height; one with a height asks for more than one without. + @Test func anOverheadRequestWithAHeightAsksForMoreThanOneWithout() throws { + let planner = GapPlanner() + let anyView = try #require(planner.plan(for: item(#"{"kind":"band","band":"overhead","span_ft":[0,3],"message":"m"}"#), leftEnd: nil, rightEnd: nil)) + let high = try #require(planner.plan(for: item(#"{"kind":"band","band":"overhead","span_ft":[0,3],"out_ft":7.0,"message":"m"}"#), leftEnd: nil, rightEnd: nil)) + #expect(anyView.asksForSameView(as: high)) + #expect(!high.asksForSameView(as: anyView)) + #expect(high.asksForSameView(as: high)) + } + /// A met past_end request moves its end on past the ground it showed, and the next one asks /// for the 2 m after that. With the end left cleared, the next request asked for the ground /// at the meter (issue #35). diff --git a/ios/HouseScanUITests/FullFlowUITests.swift b/ios/HouseScanUITests/FullFlowUITests.swift index afc9a8e38..03e31bcd9 100644 --- a/ios/HouseScanUITests/FullFlowUITests.swift +++ b/ios/HouseScanUITests/FullFlowUITests.swift @@ -113,6 +113,7 @@ final class FullFlowUITests: XCTestCase { var tape = "" var scene: [String: Any] = [:] var withdrawn: [Double]? + var requests: [String] = [] try runFlow(replay: Self.fixture, extraArguments: ["-autopilotCantGetThere"], beforeLeaving: { app, phase in guard phase == "wallWalk" else { return } // The value that matched, from one read: the map's element is rebuilt as the walk @@ -129,7 +130,13 @@ final class FullFlowUITests: XCTestCase { withdrawn = (try JSONSerialization.jsonObject(with: bytes) as? [String: Any])?["withdrawn_span_ft"] as? [Double] } scene = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + requests = try Self.serverRequests(gate: gate, test: self) }) + // #39: "I can't get there" on a request the answer raised skips only that view. The + // sample answer lists more than one view, so the upload after the skip raises the next + // one instead of showing the result. + let skippedThenMore = requests.dropLast().contains { $0 == "skipped" } + XCTAssertTrue(skippedThenMore, "no request raised after a skipped one: \(requests)") // The wall map says where each end is, in words VoiceOver reads. XCTAssertTrue(tape.contains("Left end ") && tape.contains("left of your meter") && tape.contains("Right end "), "wall map: \(tape)") @@ -163,6 +170,32 @@ final class FullFlowUITests: XCTestCase { add(attachment) } + /// #39: "Show my result" on the first request the answer raises (`-autopilotShowResult`) ends + /// the requests. The same run as `testCantGetThereEndsWhereThePhoneIs`, whose answers raise a + /// second request after a skip, here reaches the result after that one request. + @MainActor + func testShowMyResultEndsTheRequests() throws { + var requests: [String] = [] + try runFlow(replay: Self.fixture, extraArguments: ["-autopilotCantGetThere", "-autopilotShowResult"], onScene: { _, gate in + requests = try Self.serverRequests(gate: gate, test: self) + }) + XCTAssertEqual(requests, ["showResult"], "requests raised after the answer: \(requests)") + } + + /// How each request the server's answers raised ended, in order ("answered", "skipped", + /// "showResult"), from the autopilot's `server-requests.json` in the gate folder, attached + /// as `server-requests`. Empty when none was raised. + @MainActor + static func serverRequests(gate: URL, test: XCTestCase) throws -> [String] { + guard let bytes = try? Data(contentsOf: gate.appending(path: "server-requests.json")) else { return [] } + let attachment = XCTAttachment(data: bytes, uniformTypeIdentifier: "public.json") + attachment.name = "server-requests" + attachment.lifetime = .keepAlways + test.add(attachment) + let entries = try XCTUnwrap(JSONSerialization.jsonObject(with: bytes) as? [[String: Any]]) + return entries.compactMap { $0["action"] as? String } + } + /// "Something's in the way" on the spot check (`-autopilotSomethingThere`), or Continue when no /// kept photo shows the whole area: the scan stops claiming the area and goes to the server /// again, and the flow still ends on the result, which says the area was left out. The scene the last upload sent, and the one diff --git a/ios/HouseScanUITests/ScreenStatesUITests.swift b/ios/HouseScanUITests/ScreenStatesUITests.swift index 53af035ab..179077fb3 100644 --- a/ios/HouseScanUITests/ScreenStatesUITests.swift +++ b/ios/HouseScanUITests/ScreenStatesUITests.swift @@ -103,6 +103,12 @@ final class ScreenStatesUITests: XCTestCase { "result-reject": ("result.nearest", "The closest spot"), ] + /// Controls a state must offer, by identifier. + private static let controls: [String: [String]] = [ + // #39: "Show my result" on every request the check sent back, with one view left too. + "gapRequest-followUp": ["action.skipGap", "action.showResult"], + ] + /// States where the scan is packaged, so "Share scan" must show. private static let shareStates: Set = ["uploading-offline", "uploading-rejected", "result-review", "result-pass"] @@ -356,6 +362,9 @@ final class ScreenStatesUITests: XCTestCase { } XCTAssertTrue(found, "\(name): \"\(expected.text)\" is missing") } + for identifier in Self.controls[name.hasSuffix("-AX5") ? String(name.dropLast(4)) : name] ?? [] { + XCTAssertTrue(element(app, identifier).exists, "\(name): \(identifier) is missing") + } // A system banner can slide over the app mid-audit (CI's Simulator showed "Ready for Apple // Intelligence" over the photo count), so an issue fails the test only when a second // audit, after the banner's few seconds on screen, finds it again. @@ -372,6 +381,29 @@ final class ScreenStatesUITests: XCTestCase { for (_, finding) in outcome.persistent { XCTFail("\(name): \(finding.message)") } + // After the audit, so its scrolling can't change what the audit saw: a control that + // exists can still sit past the bottom edge, out of the homeowner's reach. At the default + // size it must be tappable where it is; at AX5 the screen scrolls, so after scrolling to it + // (#39: "Show my result" sits below the tape there). + for identifier in Self.controls[name.hasSuffix("-AX5") ? String(name.dropLast(4)) : name] ?? [] { + let target = element(app, identifier) + guard target.exists else { continue } + let reached = name.hasSuffix("-AX5") ? scrollUntilHittable(target, in: app) : target.isHittable + XCTAssertTrue(reached, "\(name): \(identifier) can't be tapped") + } + } + + /// Drags the screen up, at most four times, until the control can be tapped. The same slow + /// drag as `revealCutOff`, so it scrolls without momentum. + @MainActor + private func scrollUntilHittable(_ target: XCUIElement, in app: XCUIApplication) -> Bool { + let step = app.windows.firstMatch.frame.height * 0.4 + for _ in 0..<4 { + if target.isHittable { return true } + let start = app.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.7)) + start.press(forDuration: 0.1, thenDragTo: start.withOffset(CGVector(dx: 0, dy: -step)), withVelocity: .slow, thenHoldForDuration: 0.3) + } + return target.isHittable } /// At the largest text sizes a camera screen scrolls, and a control cut off by the bottom edge