From 259ca92c7c2845b2dc2845c3c11ef5e83a944697 Mon Sep 17 00:00:00 2001 From: Khari Jarrett Date: Fri, 21 Oct 2022 10:06:13 -0400 Subject: [PATCH 1/4] fix flaky test --- vision/objectdetection/detection_source.go | 17 +++++++ .../objectdetection/detection_source_test.go | 44 ++++++++++++++----- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/vision/objectdetection/detection_source.go b/vision/objectdetection/detection_source.go index 35b67534574..5a16dbedf05 100644 --- a/vision/objectdetection/detection_source.go +++ b/vision/objectdetection/detection_source.go @@ -32,6 +32,9 @@ type Source struct { cancelFunc func() } +// waitForMs tells us the number of milliseconds to wait on the channel for +const WaitForMs = 2000 + // NewSource builds the pipeline from an input VideoSource and Detector. func NewSource(src gostream.VideoSource, det Detector) (*Source, error) { // fill optional functions with identity operators @@ -75,6 +78,7 @@ func (s *Source) backgroundWorker(stream gostream.VideoStream, det Detector) { Release: release, Err: err, } + select { case <-s.cancelCtx.Done(): return @@ -96,10 +100,19 @@ func (s *Source) Read(ctx context.Context) (image.Image, func(), error) { ctx, span := trace.StartSpan(ctx, "vision::objectdetection::Source::Read") defer span.End() start := time.Now() + res, err := s.NextResult(ctx) if err != nil { return nil, nil, err } + + for res == nil { + res, err = s.NextResult(ctx) + if err != nil { + return nil, nil, err + } + } + duration := time.Since(start) fps := 1. / duration.Seconds() ovImg, err := Overlay(res.OriginalImage, res.Detections) @@ -114,6 +127,7 @@ func (s *Source) Read(ctx context.Context) (image.Image, func(), error) { func (s *Source) NextResult(ctx context.Context) (*Result, error) { ctx, span := trace.StartSpan(ctx, "vision::objectdetection::Source::NextResult") defer span.End() + select { case <-ctx.Done(): return nil, ctx.Err() @@ -121,5 +135,8 @@ func (s *Source) NextResult(ctx context.Context) (*Result, error) { return nil, s.cancelCtx.Err() case result := <-s.pipelineOutput: return result, result.Err + case <-time.After(WaitForMs * time.Millisecond): + return nil, errors.New("nothing on channel after 2 seconds") } + } diff --git a/vision/objectdetection/detection_source_test.go b/vision/objectdetection/detection_source_test.go index d8972c11b5d..8e50eb55427 100644 --- a/vision/objectdetection/detection_source_test.go +++ b/vision/objectdetection/detection_source_test.go @@ -4,6 +4,7 @@ import ( "context" "image" "testing" + "time" "go.viam.com/test" "go.viam.com/utils/artifact" @@ -20,6 +21,10 @@ func TestDetectionSource(t *testing.T) { test.That(t, err, test.ShouldBeNil) src, err := camera.NewFromReader(context.Background(), &videosource.StaticSource{ColorImg: sourceImg}, nil, camera.ColorStream) test.That(t, err, test.ShouldBeNil) + outImg, _, err := camera.ReadImage(context.Background(), src) + test.That(t, outImg, test.ShouldNotBeNil) + test.That(t, err, test.ShouldBeNil) + // make the preprocessing function p, err := objectdetection.RemoveColorChannel("b") test.That(t, err, test.ShouldBeNil) @@ -39,21 +44,38 @@ func TestDetectionSource(t *testing.T) { defer pipeline.Close() // compare with expected bounding boxes + start := time.Now() res, err := pipeline.NextResult(context.Background()) - test.That(t, err, test.ShouldBeNil) - bbs := res.Detections - test.That(t, bbs, test.ShouldHaveLength, 1) - test.That(t, bbs[0].Score(), test.ShouldEqual, 1.0) - test.That(t, bbs[0].Label(), test.ShouldEqual, "orange") - test.That(t, bbs[0].BoundingBox(), test.ShouldResemble, &image.Rectangle{image.Point{848, 424}, image.Point{999, 565}}) + tt := time.Now() + elapsed := tt.Sub(start) + if elapsed > objectdetection.WaitForMs*time.Millisecond { + test.That(t, err, test.ShouldNotBeNil) + test.That(t, res, test.ShouldBeNil) + } else { + test.That(t, err, test.ShouldBeNil) + bbs := res.Detections + test.That(t, bbs, test.ShouldHaveLength, 1) + test.That(t, bbs[0].Score(), test.ShouldEqual, 1.0) + test.That(t, bbs[0].Label(), test.ShouldEqual, "orange") + test.That(t, bbs[0].BoundingBox(), test.ShouldResemble, &image.Rectangle{image.Point{848, 424}, image.Point{999, 565}}) + } // overlay the image and see if it is red where you expect + start = time.Now() img, _, err := pipeline.Read(context.Background()) - test.That(t, err, test.ShouldBeNil) - ovImg := rimage.ConvertImage(img) - test.That(t, ovImg.GetXY(848, 424), test.ShouldResemble, rimage.Red) - test.That(t, ovImg.GetXY(998, 564), test.ShouldResemble, rimage.Red) - test.That(t, src.Close(context.Background()), test.ShouldBeNil) + tt = time.Now() + elapsed = tt.Sub(start) + if elapsed > objectdetection.WaitForMs*time.Millisecond { + test.That(t, err, test.ShouldNotBeNil) + test.That(t, img, test.ShouldBeNil) + } else { + test.That(t, err, test.ShouldBeNil) + ovImg := rimage.ConvertImage(img) + test.That(t, ovImg.GetXY(848, 424), test.ShouldResemble, rimage.Red) + test.That(t, ovImg.GetXY(998, 564), test.ShouldResemble, rimage.Red) + test.That(t, src.Close(context.Background()), test.ShouldBeNil) + } + } func TestEmptyDetection(t *testing.T) { From e6dbcad1453ccd3a82290e1759092b413744f95d Mon Sep 17 00:00:00 2001 From: Khari Jarrett Date: Fri, 21 Oct 2022 10:10:09 -0400 Subject: [PATCH 2/4] linting --- vision/objectdetection/detection_source.go | 3 +-- vision/objectdetection/detection_source_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/vision/objectdetection/detection_source.go b/vision/objectdetection/detection_source.go index 5a16dbedf05..eaf28f60ed8 100644 --- a/vision/objectdetection/detection_source.go +++ b/vision/objectdetection/detection_source.go @@ -32,7 +32,7 @@ type Source struct { cancelFunc func() } -// waitForMs tells us the number of milliseconds to wait on the channel for +// WaitForMs tells us the number of milliseconds to wait on the channel for. const WaitForMs = 2000 // NewSource builds the pipeline from an input VideoSource and Detector. @@ -138,5 +138,4 @@ func (s *Source) NextResult(ctx context.Context) (*Result, error) { case <-time.After(WaitForMs * time.Millisecond): return nil, errors.New("nothing on channel after 2 seconds") } - } diff --git a/vision/objectdetection/detection_source_test.go b/vision/objectdetection/detection_source_test.go index 8e50eb55427..bb44d7a8e35 100644 --- a/vision/objectdetection/detection_source_test.go +++ b/vision/objectdetection/detection_source_test.go @@ -75,7 +75,6 @@ func TestDetectionSource(t *testing.T) { test.That(t, ovImg.GetXY(998, 564), test.ShouldResemble, rimage.Red) test.That(t, src.Close(context.Background()), test.ShouldBeNil) } - } func TestEmptyDetection(t *testing.T) { From 5f2d8dbac5c6cad2f997f90c22d1a8ec8e962487 Mon Sep 17 00:00:00 2001 From: Khari Jarrett Date: Fri, 21 Oct 2022 10:23:28 -0400 Subject: [PATCH 3/4] change error msg --- vision/objectdetection/detection_source.go | 2 +- vision/objectdetection/detection_source_test.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/vision/objectdetection/detection_source.go b/vision/objectdetection/detection_source.go index eaf28f60ed8..e7111169f61 100644 --- a/vision/objectdetection/detection_source.go +++ b/vision/objectdetection/detection_source.go @@ -136,6 +136,6 @@ func (s *Source) NextResult(ctx context.Context) (*Result, error) { case result := <-s.pipelineOutput: return result, result.Err case <-time.After(WaitForMs * time.Millisecond): - return nil, errors.New("nothing on channel after 2 seconds") + return nil, errors.Errorf("nothing on channel after %v ms", WaitForMs) } } diff --git a/vision/objectdetection/detection_source_test.go b/vision/objectdetection/detection_source_test.go index bb44d7a8e35..7a8d18ec013 100644 --- a/vision/objectdetection/detection_source_test.go +++ b/vision/objectdetection/detection_source_test.go @@ -21,9 +21,6 @@ func TestDetectionSource(t *testing.T) { test.That(t, err, test.ShouldBeNil) src, err := camera.NewFromReader(context.Background(), &videosource.StaticSource{ColorImg: sourceImg}, nil, camera.ColorStream) test.That(t, err, test.ShouldBeNil) - outImg, _, err := camera.ReadImage(context.Background(), src) - test.That(t, outImg, test.ShouldNotBeNil) - test.That(t, err, test.ShouldBeNil) // make the preprocessing function p, err := objectdetection.RemoveColorChannel("b") From 80f29e1914a947e69b519b514e7e576b90ec1060 Mon Sep 17 00:00:00 2001 From: Khari Jarrett Date: Fri, 21 Oct 2022 11:57:43 -0400 Subject: [PATCH 4/4] remove superfluous for loop --- vision/objectdetection/detection_source.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/vision/objectdetection/detection_source.go b/vision/objectdetection/detection_source.go index e7111169f61..8d4f0b8e733 100644 --- a/vision/objectdetection/detection_source.go +++ b/vision/objectdetection/detection_source.go @@ -106,13 +106,6 @@ func (s *Source) Read(ctx context.Context) (image.Image, func(), error) { return nil, nil, err } - for res == nil { - res, err = s.NextResult(ctx) - if err != nil { - return nil, nil, err - } - } - duration := time.Since(start) fps := 1. / duration.Seconds() ovImg, err := Overlay(res.OriginalImage, res.Detections)