-
Notifications
You must be signed in to change notification settings - Fork 674
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
NOISSUE - Fix opcua-adapter events decode #951
Conversation
Codecov Report
@@ Coverage Diff @@
## master #951 +/- ##
=======================================
Coverage 83.75% 83.75%
=======================================
Files 75 75
Lines 5307 5307
=======================================
Hits 4445 4445
Misses 587 587
Partials 275 275 Continue to review full report at Codecov.
|
opcua/gopcua/sub.go
Outdated
@@ -53,16 +53,16 @@ func (b client) Subscribe(cfg opcua.Config) error { | |||
} | |||
|
|||
c := opcuaGopcua.NewClient(ep.EndpointURL, opts...) | |||
if errC := c.Connect(b.ctx); err != nil { | |||
b.logger.Error(errC.Error()) | |||
if errc := c.Connect(b.ctx); errc != nil { |
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.
Why not just follow err propagation? Why new var errc and errs?
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.
Indeed, err
is a special var in Go that can be re-assigned with :=
opcua/redis/streams.go
Outdated
@@ -89,12 +93,12 @@ func (es eventStore) Subscribe(subject string) error { | |||
} | |||
err = es.handleCreateThing(cte) | |||
case thingUpdate: | |||
ute, derr := decodeUpdateThing(event) | |||
ute, derr := decodeCreateThing(event) |
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.
derr ? why not err ?
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
opcua/gopcua/sub.go
Outdated
if errC := c.Connect(b.ctx); err != nil { | ||
b.logger.Error(errC.Error()) | ||
if err := c.Connect(b.ctx); err != nil { | ||
return fmt.Errorf("Failed to connect: %s", err) |
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.
You can use our errors
package here, something like: errors.Wrap(ErrFailedConn, err)
.
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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.
LGTM
Dismissing until Dusan's remarks are not taken into account.
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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.
LGTM!
* NOISSUE - Enable multi OPC-UA Subscriptions Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * NOISSUE - Fix opcua-adapter events decode Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Use a config file to subscribe to multiple nodes Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Add subscription config file Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Use Mainflux errors package Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
With an existing event processing implementation, whenever the processor receives a new event with some unexpected format it will break. This implementation should be able to handle unexpected metadata formats in a better way.