fix: Modify pan tilt function by getting pan/tilt range dynamically #181

presatish · 2023-02-02T22:54:47Z

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

PR Checklist

Please check if your PR fulfills the following requirements:

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?)

Testing Instructions

Signed-off-by: preethi-satishcandra <[email protected]>

brianointel · 2023-02-03T22:53:37Z

application-services/custom/camera-management/appcamera/types.go

 			} `json:"PanTilt"`
 		} `json:"PTZPosition"`
 		Token string `json:"Token"`
 	} `json:"Preset"`
 }

+type GetPTZConfigurationsResponse struct {
+	PTZConfiguration []struct {
+		DefaultAbsolutePantTiltPositionSpace  string `json:"DefaultAbsolutePantTiltPositionSpace"`


should this be 'DefaultAbsolutePanTiltPositionSpace'?

Its a typo from the onvif specification end - https://www.onvif.org/ver20/ptz/wsdl/ptz.wsdl

yeah there are actually a decent amount of typos in the specs. its suprising.

brianointel

besides my comment, looks good

ajcasagrande

1 fix and 1 suggestion

ajcasagrande · 2023-02-06T23:02:20Z

application-services/custom/camera-management/appcamera/routes.go

+	left := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Min
+	right := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Max
+	up := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Max
+	down := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Min


panTiltOffset should actually be multiplied by the range and not the min/max, like so:

Suggested change

left := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Min

right := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Max

up := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Max

down := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Min

xRange := ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Max - ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Min

right := panTiltOffset * xRange

left := -right

yRange := ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Max - ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Min

up := panTiltOffset * yRange

down := -up

ajcasagrande · 2023-02-06T23:03:56Z

application-services/custom/camera-management/appcamera/routes.go

@@ -257,6 +254,19 @@ func (app *CameraManagementApp) ptzRoute(w http.ResponseWriter, req *http.Reques
 	var res dtosCommon.BaseResponse
 	var err error

+	// Get pan/tilt x,y range for a particular device
+	ptzConfigs, err := app.getPTZConfiguration(deviceName)


It would be nice to have some caching mechanism where this only queries the camera if it has not queried it before. lazy-load style. Could be as simple as a map<deviceName, Response> and a mutex.

Signed-off-by: preethi-satishcandra <[email protected]>

ajcasagrande

Looks good. Small comments.

ajcasagrande · 2023-02-07T22:01:41Z

application-services/custom/camera-management/appcamera/routes.go

+	defer app.panTiltMutex.Unlock()
+	val, exists := app.panTiltMap[deviceName]
+	if !exists {
+		fmt.Println("Device does not exist")


we can probably remove this print

Had just added for some testing :-) will remove

ajcasagrande · 2023-02-07T22:04:01Z

application-services/custom/camera-management/appcamera/routes.go

+func (app *CameraManagementApp) getPanTiltRange(deviceName string) (PanTiltRange, error) {
+	app.panTiltMutex.Lock()
+	defer app.panTiltMutex.Unlock()
+	val, exists := app.panTiltMap[deviceName]


I would probably call val as panTiltRange instead, and then return panTiltRange on line 329, and on line 322, change panTiltRange := to panTiltRange = , and remove the return on line 327. the code will fall out the if and return through one path.

Signed-off-by: preethi-satishcandra <[email protected]>

ajcasagrande

LGTM!

ChristianDarr-personal

LGTM

Signed-off-by: Bryon Nevis <[email protected]> Signed-off-by: Bryon Nevis <[email protected]> fix: remove duplicated path from query uri (edgexfoundry#182) Signed-off-by: Anthony Casagrande <[email protected]> fix: Modify pan tilt function by getting pan/tilt range dynamically (edgexfoundry#181) * fix: Get pan/tilt values dynamically Signed-off-by: preethi-satishcandra <[email protected]> feat: support for usb cameras and onvif zoom functionality in camera mgmt app (edgexfoundry#184) Signed-off-by: Anthony Casagrande <[email protected]> Signed-off-by: Christian Darr <[email protected]> Signed-off-by: Brian Osburn <[email protected]> Co-authored-by: Brian Osburn <[email protected]> Co-authored-by: Christian Darr <[email protected]> feat: updated app service with target type Signed-off-by: brianointel <[email protected]> feat: added code for pipeline function Signed-off-by: brianointel <[email protected]> feat: template code for funtion pipelines by topic Signed-off-by: brianointel <[email protected]> feat: added initial event handler Signed-off-by: brianointel <[email protected]> feat: more device messagebus progress Signed-off-by: brianointel <[email protected]> feat: message bus code Signed-off-by: Brian Osburn <[email protected]> feat: messagebus progress Signed-off-by: Brian Osburn <[email protected]> feat: created defaultpipelinefunction Signed-off-by: Brian Osburn <[email protected]> feat: try to start pipelines on startup Signed-off-by: brianointel <[email protected]> feat: small changes Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: added config params Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: handle device delete event to stop pipeline also some minor refactoring Signed-off-by: Anthony Casagrande <[email protected]>

* feat: Update device service example for Levski Signed-off-by: Leonard Goodell <[email protected]> build: Update HELM Chart to Levski released images (edgexfoundry#140) Signed-off-by: Leonard Goodell <[email protected]> Signed-off-by: Leonard Goodell <[email protected]> feat: added support for shared and non-shared volumes Close edgexfoundry#92 Signed-off-by: Kyle Morton <[email protected]> fix: EdgeX UI in kubernetes was not binding container network IF Signed-off-by: Bryon Nevis <[email protected]> feat: Added support for onvif and usb camera Closes edgexfoundry#107 Signed-off-by: Kyle Morton <[email protected]> feat: Added support for modbus-simulator Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-mqtt and mqtt-broker Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-modbus Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-bacnet Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-snmp Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-rfid-llrp Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for ds-gpio Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for asc-mqtt and asc-httpexport Signed-off-by: Kyle Morton <[email protected]> feat: Add support in helm for asc-sample, asc-metrics, and as-llrp Signed-off-by: Kyle Morton <[email protected]> fix: Updated device-onvif-camera and device-usb-camera directory and volume (edgexfoundry#156) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> fix: added service for edgex-ui (edgexfoundry#162) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> feat: Added support for nats (edgexfoundry#163) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> fix: Updated health checks to appropriate protocol (edgexfoundry#164) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> fix: added kuiper-connections and kuipers-sources volumes (edgexfoundry#165) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> fix: Correct addConsulRoles list (edgexfoundry#166) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> feat: Support optional hostPort binding (edgexfoundry#167) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> feat: Expose services on a per-service basis (edgexfoundry#169) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> fix: Added container port names (edgexfoundry#170) Signed-off-by: Kyle Morton <[email protected]> Signed-off-by: Kyle Morton <[email protected]> Co-authored-by: Kyle Morton <[email protected]> feat!: Remove system-mgmt-agent (deprecated) (edgexfoundry#175) BREAKING CHANGE: Remove system-mgmt-agent (deprectated) Signed-off-by: Bryon Nevis <[email protected]> Signed-off-by: Bryon Nevis <[email protected]> docs: add link to camera app example video Signed-off-by: Anthony Casagrande <[email protected]> feat: Add Helm security scanners (edgexfoundry#174) Signed-off-by: Bryon Nevis <[email protected]> Signed-off-by: Bryon Nevis <[email protected]> fix: remove duplicated path from query uri (edgexfoundry#182) Signed-off-by: Anthony Casagrande <[email protected]> fix: Modify pan tilt function by getting pan/tilt range dynamically (edgexfoundry#181) * fix: Get pan/tilt values dynamically Signed-off-by: preethi-satishcandra <[email protected]> feat: support for usb cameras and onvif zoom functionality in camera mgmt app (edgexfoundry#184) Signed-off-by: Anthony Casagrande <[email protected]> Signed-off-by: Christian Darr <[email protected]> Signed-off-by: Brian Osburn <[email protected]> Co-authored-by: Brian Osburn <[email protected]> Co-authored-by: Christian Darr <[email protected]> feat: updated app service with target type Signed-off-by: brianointel <[email protected]> feat: added code for pipeline function Signed-off-by: brianointel <[email protected]> feat: template code for funtion pipelines by topic Signed-off-by: brianointel <[email protected]> feat: added initial event handler Signed-off-by: brianointel <[email protected]> feat: more device messagebus progress Signed-off-by: brianointel <[email protected]> feat: message bus code Signed-off-by: Brian Osburn <[email protected]> feat: messagebus progress Signed-off-by: Brian Osburn <[email protected]> feat: created defaultpipelinefunction Signed-off-by: Brian Osburn <[email protected]> feat: try to start pipelines on startup Signed-off-by: brianointel <[email protected]> feat: small changes Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: added config params Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: handle device delete event to stop pipeline Signed-off-by: brianointel <[email protected]> also some minor refactoring Signed-off-by: Anthony Casagrande <[email protected]> fix: pr updates Signed-off-by: Brian Osburn <[email protected]> refactor: fix imports from merge Signed-off-by: Anthony Casagrande <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> fix: updated funtion name Signed-off-by: Brian Osburn <[email protected]> feat: Add Helm security scanners (edgexfoundry#174) Signed-off-by: Bryon Nevis <[email protected]> Signed-off-by: Bryon Nevis <[email protected]> fix: remove duplicated path from query uri (edgexfoundry#182) Signed-off-by: Anthony Casagrande <[email protected]> fix: Modify pan tilt function by getting pan/tilt range dynamically (edgexfoundry#181) * fix: Get pan/tilt values dynamically Signed-off-by: preethi-satishcandra <[email protected]> feat: support for usb cameras and onvif zoom functionality in camera mgmt app (edgexfoundry#184) Signed-off-by: Anthony Casagrande <[email protected]> Signed-off-by: Christian Darr <[email protected]> Signed-off-by: Brian Osburn <[email protected]> Co-authored-by: Brian Osburn <[email protected]> Co-authored-by: Christian Darr <[email protected]> feat: updated app service with target type Signed-off-by: brianointel <[email protected]> feat: added code for pipeline function Signed-off-by: brianointel <[email protected]> feat: template code for funtion pipelines by topic Signed-off-by: brianointel <[email protected]> feat: added initial event handler Signed-off-by: brianointel <[email protected]> feat: more device messagebus progress Signed-off-by: brianointel <[email protected]> feat: message bus code Signed-off-by: Brian Osburn <[email protected]> feat: messagebus progress Signed-off-by: Brian Osburn <[email protected]> feat: created defaultpipelinefunction Signed-off-by: Brian Osburn <[email protected]> feat: try to start pipelines on startup Signed-off-by: Brian Osburn <[email protected]> feat: small changes Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: added config params Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> feat: handle device delete event to stop pipeline also some minor refactoring Signed-off-by: Anthony Casagrande <[email protected]> fix: pr updates Signed-off-by: Brian Osburn <[email protected]> refactor: fix imports from merge Signed-off-by: Anthony Casagrande <[email protected]> feat: pr updates Signed-off-by: Brian Osburn <[email protected]> fix: updated funtion name Signed-off-by: Brian Osburn <[email protected]> fix: corrected error message placement Signed-off-by: brianointel <[email protected]>

presatish added 2 commits February 1, 2023 22:57

fix: Get pan/tilt values dynamically

1ee070d

Signed-off-by: preethi-satishcandra <[email protected]>

fix: clean up of the code

c2b07da

Signed-off-by: preethi-satishcandra <[email protected]>

brianointel reviewed Feb 3, 2023

View reviewed changes

brianointel previously approved these changes Feb 3, 2023

View reviewed changes

ajcasagrande requested changes Feb 6, 2023

View reviewed changes

fix: code review comments addressed

b4315f3

Signed-off-by: preethi-satishcandra <[email protected]>

presatish dismissed brianointel’s stale review via b4315f3 February 7, 2023 21:13

ajcasagrande requested changes Feb 7, 2023

View reviewed changes

fix: more code review comments addressed

428a354

Signed-off-by: preethi-satishcandra <[email protected]>

ajcasagrande approved these changes Feb 7, 2023

View reviewed changes

ChristianDarr-personal approved these changes Feb 7, 2023

View reviewed changes

lenny-goodell approved these changes Feb 8, 2023

View reviewed changes

lenny-goodell merged commit a843b28 into edgexfoundry:main Feb 8, 2023

ajcasagrande deleted the modify-pan-tilt branch February 13, 2023 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Modify pan tilt function by getting pan/tilt range dynamically #181

fix: Modify pan tilt function by getting pan/tilt range dynamically #181

presatish commented Feb 2, 2023

brianointel Feb 3, 2023

presatish Feb 6, 2023

ajcasagrande Feb 6, 2023

brianointel left a comment

ajcasagrande left a comment

ajcasagrande Feb 6, 2023

ajcasagrande Feb 6, 2023

ajcasagrande left a comment

ajcasagrande Feb 7, 2023

presatish Feb 7, 2023

ajcasagrande Feb 7, 2023

ajcasagrande left a comment

ChristianDarr-personal left a comment

-	left := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Min
-	right := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Max
-	up := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Max
-	down := panTiltOffset * ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Min
+	xRange := ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Max - ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.XRange.Min
+	right := panTiltOffset * xRange
+	left := -right
+	yRange := ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Max - ptzConfigs.PTZConfiguration[0].PanTiltLimits.Range.YRange.Min
+	up := panTiltOffset * yRange
+	down := -up

fix: Modify pan tilt function by getting pan/tilt range dynamically #181

fix: Modify pan tilt function by getting pan/tilt range dynamically #181

Conversation

presatish commented Feb 2, 2023

PR Checklist

Testing Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianointel left a comment

Choose a reason for hiding this comment

ajcasagrande left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajcasagrande left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ajcasagrande left a comment

Choose a reason for hiding this comment

ChristianDarr-personal left a comment

Choose a reason for hiding this comment