-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENG-2320] Reduce cyclomatic complexity of the browse function #119
Open
kanapuli
wants to merge
10
commits into
master
Choose a base branch
from
eng-2320-simplify-attributes
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
ee59574
Simply Attribute handling
kanapuli 2a84db2
Simplify newPath
kanapuli 49c021a
Create new node_attributes.go
kanapuli d928193
Simplify attributes in an idiomatic way for more readablity
kanapuli 7bf12e7
Fix tests. Format error in a better way
kanapuli 495c73a
Include better comments
kanapuli be42331
More comments to node attributes.go
kanapuli 6c3eb0a
Remove code setting node class to an object when accessleveltype is none
kanapuli 2c5ecdd
Address PR comments
kanapuli c962124
Fix tests
kanapuli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| package opcua_plugin | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/gopcua/opcua/id" | ||
| "github.com/gopcua/opcua/ua" | ||
| ) | ||
|
|
||
| // AttributeHandler is the struct that handles the attributes of a node | ||
| type AttributeHandler struct { | ||
| // Using more idiomatic names | ||
| handleOK func(value *ua.Variant) error // handleOK is the function that handles the OK status | ||
| handleNotReadable func() // handleNotReadable is the function that handles the NotReadable status | ||
| ignoreInvalidAttr bool // ignoreInvalidAttr is the flag that determines if invalid attributes should be ignored | ||
| requiresValue bool // requiresValue is the flag that determines if a value is required | ||
| affectsNodeClass bool // affectsNodeClass is the flag that determines if the node class should be affected | ||
| } | ||
|
|
||
| // handleAttributeStatus is the function that handles the attributes of a node | ||
| func handleAttributeStatus( | ||
| name string, | ||
| attr *ua.DataValue, | ||
| def *NodeDef, | ||
| path string, | ||
| logger Logger, | ||
| handler AttributeHandler, | ||
| ) error { | ||
| if attr == nil { | ||
| return fmt.Errorf("attribute is nil for node: %s and attribute: %s", def.NodeID, name) | ||
| } | ||
|
|
||
| if attr.Status == ua.StatusOK { | ||
| if attr.Value == nil && handler.requiresValue { | ||
| return fmt.Errorf("attribute value is nil for node: %s and attribute: %s", def.NodeID, name) | ||
| } | ||
| return handleOKStatus(attr.Value, handler) | ||
| } | ||
|
|
||
| if attr.Status == ua.StatusBadSecurityModeInsufficient { | ||
| return fmt.Errorf("got insufficient security mode for node: %s and attribute: %s", def.NodeID, name) | ||
| } | ||
|
|
||
| if attr.Status == ua.StatusBadAttributeIDInvalid { | ||
| if handler.ignoreInvalidAttr { | ||
| return nil | ||
| } | ||
| return fmt.Errorf("invalid attribute ID for node: %s and attribute: %s", def.NodeID, name) | ||
| } | ||
|
|
||
| if attr.Status == ua.StatusBadNotReadable { | ||
| handleNotReadable(def, handler, path, logger) | ||
| return nil | ||
| } | ||
|
|
||
| return attr.Status | ||
| } | ||
|
|
||
| // handleOKStatus processes successful attribute reads | ||
| func handleOKStatus(value *ua.Variant, handler AttributeHandler) error { | ||
| if handler.handleOK != nil && value != nil { | ||
| return handler.handleOK(value) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // handleNotReadable processes non-readable attributes | ||
| func handleNotReadable(def *NodeDef, handler AttributeHandler, path string, logger Logger) { | ||
| if handler.affectsNodeClass { | ||
| def.NodeClass = ua.NodeClassObject | ||
| } | ||
| if handler.handleNotReadable != nil { | ||
| handler.handleNotReadable() | ||
| } | ||
| logger.Warnf("Access denied for node: %s, continuing...\n", path) | ||
| } | ||
|
|
||
| // processNodeAttributes processes all attributes for a node | ||
| // This function is used to process the attributes of a node and set the NodeDef struct | ||
| // def.NodeClass, def.BrowseName, def.Description, def.AccessLevel, def.DataType are set in the processNodeAttributes function | ||
| func processNodeAttributes(attrs []*ua.DataValue, def *NodeDef, path string, logger Logger) error { | ||
| // NodeClass (attrs[0]) | ||
| nodeClassHandler := AttributeHandler{ | ||
| handleOK: func(value *ua.Variant) error { | ||
| def.NodeClass = ua.NodeClass(value.Int()) | ||
| return nil | ||
| }, | ||
| requiresValue: true, // NodeClass is required else the node is not valid | ||
| affectsNodeClass: true, // If the node class could not be determined, it is set to Object type and browsing can be continued | ||
| } | ||
| if err := handleAttributeStatus("NodeClass", attrs[0], def, path, logger, nodeClassHandler); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // BrowseName (attrs[1]) | ||
| browseNameHandler := AttributeHandler{ | ||
| handleOK: func(value *ua.Variant) error { | ||
| def.BrowseName = value.String() | ||
| return nil | ||
| }, | ||
| requiresValue: true, // BrowseName is required else the node is not valid | ||
| } | ||
| if err := handleAttributeStatus("BrowseName", attrs[1], def, path, logger, browseNameHandler); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Description (attrs[2]) | ||
| descriptionHandler := AttributeHandler{ | ||
| handleOK: func(value *ua.Variant) error { | ||
| if value != nil { | ||
| def.Description = value.String() | ||
| return nil | ||
| } | ||
| // Description for Kepware v6 is OPCUATYPE_NULL. Set to empty string in such cases | ||
| def.Description = "" | ||
| return nil | ||
| }, | ||
| ignoreInvalidAttr: true, // For some node, Description is not available and this is not an error and can be ignored | ||
| affectsNodeClass: true, // If the node class could not be determined, it is set to Object type and browsing can be continued. By setting it to object, we will not subscribe to this node | ||
| } | ||
| if err := handleAttributeStatus("Description", attrs[2], def, path, logger, descriptionHandler); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // AccessLevel (attrs[3]) | ||
| accessLevelHandler := AttributeHandler{ | ||
| handleOK: func(value *ua.Variant) error { | ||
| def.AccessLevel = ua.AccessLevelType(value.Int()) | ||
| return nil | ||
| }, | ||
| ignoreInvalidAttr: true, // For some node, AccessLevel is not available and this is not an error and can be ignored | ||
| } | ||
| if err := handleAttributeStatus("AccessLevel", attrs[3], def, path, logger, accessLevelHandler); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // DataType (attrs[4]) | ||
| dataTypeHandler := AttributeHandler{ | ||
| handleOK: func(value *ua.Variant) error { | ||
| if value == nil { | ||
| logger.Debugf("ignoring node: %s as its datatype is nil...\n", path) | ||
| return fmt.Errorf("datatype is nil for node: %s and attribute: %s", def.NodeID, "DataType") | ||
| } | ||
| def.DataType = getDataTypeString(value.NodeID().IntID()) | ||
| return nil | ||
| }, | ||
| ignoreInvalidAttr: true, // For some node, DataType is not available and this is not an error and can be ignored | ||
| affectsNodeClass: true, // If the node class could not be determined, it is set to Object type and browsing can be continued. By setting it to object, we will not subscribe to this node | ||
| } | ||
| if err := handleAttributeStatus("DataType", attrs[4], def, path, logger, dataTypeHandler); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| var dataTypeMap = map[uint32]string{ | ||
| id.DateTime: "time.Time", | ||
| id.Boolean: "bool", | ||
| id.SByte: "int8", | ||
| id.Int16: "int16", | ||
| id.Int32: "int32", | ||
| id.Byte: "byte", | ||
| id.UInt16: "uint16", | ||
| id.UInt32: "uint32", | ||
| id.UtcTime: "time.Time", | ||
| id.String: "string", | ||
| id.Float: "float32", | ||
| id.Double: "float64", | ||
| } | ||
|
|
||
| func getDataTypeString(typeID uint32) string { | ||
| if dtype, ok := dataTypeMap[typeID]; ok { | ||
| return dtype | ||
| } | ||
| return fmt.Sprintf("ns=%d;i=%d", 0, typeID) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be careful about warning, they will prevent deployment in the management console
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing code in master branch uses logger.Warnf for this scenario and the status quo is maintained.