Skip to content

show progress arc while waiting for first map and clean up atomic state#17

Merged
JohnN193 merged 10 commits intomainfrom
pr-c-state-cleanup
Mar 23, 2026
Merged

show progress arc while waiting for first map and clean up atomic state#17
JohnN193 merged 10 commits intomainfrom
pr-c-state-cleanup

Conversation

@JohnN193
Copy link
Collaborator

Two related changes: a user-visible feature and the internal state refactor it motivated.

Progress arc
While a job is running but no map has arrived yet, PointCloudMap returns a point cloud arc instead of the blank default. The arc grows from a small sliver to a full circle over 5 minutes, giving visual feedback that the session is active. (Text label added in the next PR.)

Atomic state cleanup

  • Merge activeJob (job ID string) + jobStartTime into a single activeJobState atomic struct — both fields are now updated in one atomic swap, eliminating the window where one is set and the other isn't. Nil pointer replaces the empty-string sentinel.
  • Wrap updatingMapName / updatingMapVersion into an updatingMapInfo struct; nil replaces the != "" guard and fixes a latent bug where version could be set alongside an empty name.
  • Clear activeJob in StopJob to stop the polling loop, while preserving lastPointCloudURL so the final map stays visible after the session ends.
  • machine_part_id is now required at startup (matching machine_id behavior) — safe now that it's auto-populated from the env var.
  • Store logger on AppClient at construction; removes the logger parameter from CheckSensorsDataCapture.

Fix

  • Add page=slam to the StopJob return URL (without it the app backend redirects to /fleet/location/...).

Part of a series: [PR A] → [PR B] → PR C → [PR D]

@JohnN193 JohnN193 mentioned this pull request Mar 20, 2026
@JohnN193 JohnN193 force-pushed the pr-c-state-cleanup branch 2 times, most recently from c2df93b to 71b5798 Compare March 20, 2026 19:58
@JohnN193 JohnN193 force-pushed the pr-b-config branch 2 times, most recently from 6a13444 to de03d7f Compare March 20, 2026 20:08
@JohnN193 JohnN193 force-pushed the pr-c-state-cleanup branch from 71b5798 to 151372c Compare March 20, 2026 20:08
@JohnN193 JohnN193 force-pushed the pr-c-state-cleanup branch from 151372c to 6d5fc3d Compare March 20, 2026 20:16
// generateProgressRingPCD generates a point cloud of a progress arc indicating elapsed time.
// The arc grows clockwise from 0 to a full circle over progressRingDuration, giving the user
// visual feedback while waiting for the first cloudslam map to appear.
func generateProgressRingPCD(elapsed time.Duration) ([]byte, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

when running cloudslam there is always a period of waiting for the job to start, because we have to spin up a job and setup the machine.

now we display something that moves, and in the next pr we expand on that further


pc := pointcloud.NewBasicEmpty()
for i := 0; i < filledPoints; i++ {
angle := float64(i) / numPoints * 2 * math.Pi

Choose a reason for hiding this comment

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

aren't you duplicating work here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

kk going to update the code to store the pointcloud in state rather than recreate everytime

}

var buf bytes.Buffer
if err := pointcloud.ToPCD(pc, &buf, pointcloud.PCDAscii); err != nil {

Choose a reason for hiding this comment

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

ascii is large. could do binary

Base automatically changed from pr-b-config to main March 23, 2026 14:40
JohnN193 and others added 6 commits March 23, 2026 10:41
While a job is active but no map has been received yet, PointCloudMap
returns a PCD point arc instead of the blank default. The arc grows from
a small sliver to a full circle over 5 minutes, giving the user visual
feedback that the session is running.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- merge activeJob + jobStartTime into activeJobState struct; nil pointer
  replaces empty-string sentinel, and startedAt replaces the separate
  jobStartTime atomic so both fields are updated atomically
- wrap updatingMapName/updatingMapVersion into updatingMapInfo struct; nil
  replaces the != "" guard and fixes a latent bug where version could be
  set with an empty name
- clear activeJob in StopJob (stops polling) while keeping lastPointCloudURL
  so the final map stays visible after a session ends
- store logger on AppClient at construction; drop logger param from
  CheckSensorsDataCapture

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
part_id is now required (from config or env var), matching the existing
machine_id behavior. removes the dead partID != "" guard in initialize.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@JohnN193 JohnN193 force-pushed the pr-c-state-cleanup branch from 6d5fc3d to 3280815 Compare March 23, 2026 14:42
JohnN193 and others added 2 commits March 23, 2026 10:53
- ParseSensorsForPackage now delegates to sensorInfoToProto instead of
  re-iterating svc.sensors
- generateProgressRingPCD uses PCDBinary instead of PCDAscii for smaller output

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Background thread now owns the current PCD entirely: builds the
  progress arc incrementally (appending only new points per tick) while
  waiting for a real map, then downloads and stores map bytes once
  available. PointCloudMap just reads the stored bytes.
- Removes lastPointCloudURL and progressPCD atomics in favor of a single
  currentPCD atomic.Pointer[[]byte]
- ParseSensorsForPackage delegates to sensorInfoToProto to avoid
  duplicating sensor iteration
- generateProgressRingPCD uses PCDBinary instead of PCDAscii

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mapURL := resp.GetMapUrl()
svc.lastPointCloudURL.Store(&mapURL)

if mapURL := resp.GetMapUrl(); mapURL != "" {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved all of the pcd generation logic to the background thread, so now we don't have multiple threads making pcds

the background thread builds them, and GetMap sends them

@JohnN193 JohnN193 requested a review from nfranczak March 23, 2026 15:27
}
} else if !job.startedAt.IsZero() {
// no map yet — incrementally build the progress arc
fraction := math.Max(0.02, math.Min(time.Since(job.startedAt).Seconds()/arcProgressRingDuration.Seconds(), 1.0))

Choose a reason for hiding this comment

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

0.02?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

0.02 so we always start with some % of the circle showing up

}
} else if !job.startedAt.IsZero() {
// no map yet — incrementally build the progress arc
fraction := math.Max(minArcPercent, math.Min(time.Since(job.startedAt).Seconds()/arcProgressRingDuration.Seconds(), 1.0))

Choose a reason for hiding this comment

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

1.0?

angle := float64(i) / arcNumPoints * 2 * math.Pi
x := arcRadius * math.Cos(angle)
y := arcRadius * math.Sin(angle)
if err := arcPC.Set(r3.Vector{X: x, Y: y, Z: 0}, pointcloud.NewBasicData()); err != nil {

Choose a reason for hiding this comment

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

opt: 0 should be a const

Copy link

@nfranczak nfranczak left a comment

Choose a reason for hiding this comment

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

just the optional to have

arcPC.Set(r3.Vector{X: x, Y: y, Z: 0}

become

arcPC.Set(r3.Vector{X: x, Y: y, Z: someConstName}

@JohnN193 JohnN193 merged commit cb67565 into main Mar 23, 2026
1 check passed
@JohnN193 JohnN193 deleted the pr-c-state-cleanup branch March 23, 2026 18:16
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