Skip to content

fix: error forwarding in startStreaming#113

Merged
lenny-goodell merged 3 commits intoedgexfoundry:mainfrom
EdgeX-Camera-Management:error-forwarding
Nov 14, 2022
Merged

fix: error forwarding in startStreaming#113
lenny-goodell merged 3 commits intoedgexfoundry:mainfrom
EdgeX-Camera-Management:error-forwarding

Conversation

@ChristianDarr-personal
Copy link
Copy Markdown
Contributor

Closes #23

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

Make sure that the service is not streaming. Then, execute the startStreaming command, making sure that it should create an error (invalid input, etc). The rest response should be an error code with a description of the error.

New Dependency Instructions (If applicable)

Signed-off-by: Darr, Christian <christian.darr@intel.com>
Signed-off-by: Darr, Christian <christian.darr@intel.com>
Signed-off-by: Darr, Christian <christian.darr@intel.com>
Copy link
Copy Markdown
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

@cloudxxx8 cloudxxx8 requested a review from FelixTing November 11, 2022 06:33
@farshidtz farshidtz self-requested a review November 11, 2022 06:41
Copy link
Copy Markdown
Member

@farshidtz farshidtz left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the fix!

I tested and could not reproduce the bug described in #23. It adds 1s delay to start requests, which is by design.

$ time curl -X PUT -d '{                                                                              
    "StartStreaming": {
      "InputImageSize": "640x480",
      "OutputVideoQuality": "5"
    }
}' http://localhost:59882/api/v2/device/name/example-camera/StartStreaming
{"apiVersion":"v2","statusCode":200}

real	0m1.011s
user	0m0.004s
sys	0m0.004s

$ snap stop edgex-device-usb-camera.rtsp-simple-server 
Stopped.

$ time curl -X PUT -d '{                                                                              
    "StartStreaming": {
      "InputImageSize": "640x480",
      "OutputVideoQuality": "5"
    }
}' http://localhost:59882/api/v2/device/name/example-camera/StartStreaming
{"apiVersion":"v2","message":"request failed, status code: 500, err: {\"apiVersion\":\"v2\",\"message\":\"error writing DeviceResourece StartStreaming for example-camera -\\u003e the video streaming process for device example-camera has stopped, error: Failed Finish FFMPEG ([-nostats -loglevel 0 -y -s 640x480 -i /dev/video0 -qscale 5 -f rtsp -hls_list_size 0 rtsp://localhost:8554/stream/example-camera]) with exit status 1 message   -\\u003e Failed Finish FFMPEG ([-nostats -loglevel 0 -y -s 640x480 -i /dev/video0 -qscale 5 -f rtsp -hls_list_size 0 rtsp://localhost:8554/stream/example-camera]) with exit status 1 message  \",\"statusCode\":500}","statusCode":500}

real	0m0.518s
user	0m0.000s
sys	0m0.007s

@lenny-goodell lenny-goodell merged commit 02bc335 into edgexfoundry:main Nov 14, 2022
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.

StartStreaming errors not passed to the caller

4 participants