Skip to content

Conversation

cedricve
Copy link
Member

@cedricve cedricve commented Sep 17, 2025

Description

Pull Request: hotfix/v3.5.6

Motivation

The primary motivation for this hotfix is to address compatibility and stability issues by downgrading the github.com/kerberos-io/onvif dependency from version v1.0.0 to v0.0.7. The newer version of this dependency introduced breaking changes that affected the functionality of our ONVIF integration. Additionally, several unused dependencies have been removed to streamline the project and reduce potential maintenance overhead.

Why It Improves the Project

  1. Stability and Compatibility: Downgrading github.com/kerberos-io/onvif to v0.0.7 ensures that our project remains stable and compatible with the existing codebase, preventing runtime errors and unexpected behavior related to ONVIF device interactions.
  2. Code Cleanup: Removing unused dependencies such as github.com/clbanning/mxj/v2, github.com/icholy/digest, and github.com/juju/errors reduces the project's complexity and potential security vulnerabilities, making the codebase easier to maintain.
  3. Performance: By commenting out and removing non-essential ONVIF-related code, we can improve the performance and reliability of the system, as unnecessary operations and dependencies are eliminated.
  4. Maintainability: Simplifying the go.mod and go.sum files by removing obsolete dependencies helps in maintaining a clean and manageable project structure.

This hotfix ensures that the project remains robust and maintainable, addressing immediate issues while setting a foundation for future enhancements.

@Copilot Copilot AI review requested due to automatic review settings September 17, 2025 14:09
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This is a hotfix release (v3.5.6) that removes ONVIF event handling functionality and simplifies the ONVIF library integration by downgrading from v1.0.0 to v0.0.7.

  • Commented out all ONVIF event-related code including digital input/output handling and relay triggering
  • Simplified ONVIF device connection by removing capabilities parameter from ConnectToOnvifDevice function
  • Updated ONVIF library dependency from v1.0.0 to v0.0.7 with corresponding code adjustments

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
machinery/src/routers/mqtt/main.go Comments out HandleTriggerRelay function entirely
machinery/src/routers/http/methods.go Removes unused log import and comments out digital input/output HTTP handlers
machinery/src/onvif/main.go Major refactor removing event handling, simplifying device connection, and updating to older ONVIF library API
machinery/src/cloud/Cloud.go Comments out ONVIF event subscription code in heartbeat handling
machinery/main.go Comments out ONVIF device discovery functionality
machinery/go.mod Downgrades kerberos-io/onvif from v1.0.0 to v0.0.7 and removes related dependencies

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +759 to +786
if configurations.PTZConfiguration.DefaultAbsolutePantTiltPositionSpace != "" {
functions = append(functions, "AbsolutePanTiltMove")
canPanTilt = true
}
if configurations.PTZConfiguration[0].DefaultAbsoluteZoomPositionSpace != nil {
if configurations.PTZConfiguration.DefaultAbsoluteZoomPositionSpace != "" {
functions = append(functions, "AbsoluteZoomMove")
canZoom = true
}
if configurations.PTZConfiguration[0].DefaultRelativePanTiltTranslationSpace != nil {
if configurations.PTZConfiguration.DefaultRelativePanTiltTranslationSpace != "" {
functions = append(functions, "RelativePanTiltMove")
canPanTilt = true
}
if configurations.PTZConfiguration[0].DefaultRelativeZoomTranslationSpace != nil {
if configurations.PTZConfiguration.DefaultRelativeZoomTranslationSpace != "" {
functions = append(functions, "RelativeZoomMove")
canZoom = true
}
if configurations.PTZConfiguration[0].DefaultContinuousPanTiltVelocitySpace != nil {
if configurations.PTZConfiguration.DefaultContinuousPanTiltVelocitySpace != "" {
functions = append(functions, "ContinuousPanTiltMove")
canPanTilt = true
}
if configurations.PTZConfiguration[0].DefaultContinuousZoomVelocitySpace != nil {
if configurations.PTZConfiguration.DefaultContinuousZoomVelocitySpace != "" {
functions = append(functions, "ContinuousZoomMove")
canZoom = true
}
if configurations.PTZConfiguration[0].DefaultPTZSpeed != nil {
if configurations.PTZConfiguration.DefaultPTZSpeed != nil {
functions = append(functions, "PTZSpeed")
}
if configurations.PTZConfiguration[0].DefaultPTZTimeout != nil {
if configurations.PTZConfiguration.DefaultPTZTimeout != "" {
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

Accessing PTZConfiguration as a single object instead of an array. Based on the ONVIF library structure, this should likely be configurations.PTZConfiguration[0].DefaultAbsolutePantTiltPositionSpace to access the first configuration.

Copilot uses AI. Check for mistakes.

Comment on lines +306 to +311
Space: configuration.PTZConfiguration.DefaultAbsolutePantTiltPositionSpace,
}

absoluteZoomVector := xsdonvif.Vector1D{
absoluteZoomVector := xsd.Vector1D{
X: zoom,
Space: configuration.PTZConfiguration[0].DefaultAbsoluteZoomPositionSpace,
Space: configuration.PTZConfiguration.DefaultAbsoluteZoomPositionSpace,
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

Same issue as above - accessing PTZConfiguration as a single object instead of an array element. This should be configuration.PTZConfiguration[0].DefaultAbsolutePantTiltPositionSpace.

Copilot uses AI. Check for mistakes.

Comment on lines +317 to +318
PanTilt: absolutePantiltVector,
Zoom: absoluteZoomVector,
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The PTZVector struct expects pointer fields based on the original code structure. These should be PanTilt: &absolutePantiltVector and Zoom: &absoluteZoomVector.

Suggested change
PanTilt: absolutePantiltVector,
Zoom: absoluteZoomVector,
PanTilt: &absolutePantiltVector,
Zoom: &absoluteZoomVector,

Copilot uses AI. Check for mistakes.

_, err := device.CallMethod(ptz.ContinuousMove{
ProfileToken: &token,
Velocity: xsdonvif.PTZSpeedZoom{
ProfileToken: token,
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The ProfileToken field expects a pointer to the token. This should be ProfileToken: &token.

Suggested change
ProfileToken: token,
ProfileToken: &token,

Copilot uses AI. Check for mistakes.

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.

1 participant