Skip to content
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 opc-ua message type handling #1071

Merged
merged 5 commits into from
Mar 12, 2020
Merged

NOISSUE - Fix opc-ua message type handling #1071

merged 5 commits into from
Mar 12, 2020

Conversation

manuio
Copy link
Contributor

@manuio manuio commented Mar 11, 2020

Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@manuio manuio requested a review from a team as a code owner March 11, 2020 18:39
@@ -24,6 +24,9 @@ const (

defOffset = 0
defLimit = 10

defNamespace = "ns=0"
defIdentifier = "i=84"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please leave a comment next to these magic numbers.

@@ -14,7 +14,7 @@ import (
"github.com/mainflux/mainflux/opcua"
)

const maxChildrens = 7
const maxChildrens = 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here - why 5, some comment please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't really explain why 5 or 7... It's just to try to make this faster but it depends on the server...

Copy link
Contributor

Choose a reason for hiding this comment

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

cant this be in env var

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will remove this and change the architecture. No need for now

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
@codecov-io
Copy link

codecov-io commented Mar 11, 2020

Codecov Report

Merging #1071 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1071      +/-   ##
==========================================
- Coverage   78.27%   78.24%   -0.04%     
==========================================
  Files          95       95              
  Lines        6648     6648              
==========================================
- Hits         5204     5202       -2     
- Misses       1134     1136       +2     
  Partials      310      310              
Impacted Files Coverage Δ
things/service.go 88.97% <0.00%> (-1.48%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba97e86...37bef5c. Read the comment docs.

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
nodes = append(nodes, children...)
}
return nil
bc, err := broseChildren(n, def.Path, level, id.HasComponent)
Copy link
Contributor

Choose a reason for hiding this comment

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

browse

@@ -14,7 +14,7 @@ import (
"github.com/mainflux/mainflux/opcua"
)

const maxChildrens = 7
const maxChildrens = 5
Copy link
Contributor

Choose a reason for hiding this comment

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

cant this be in env var

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@drasko drasko merged commit b3f91f5 into absmach:master Mar 12, 2020
manuio added a commit that referenced this pull request Oct 12, 2020
* NOISSUE - Fix opc-ua message type handling

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Add comments

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix reviews

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Fix typo

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>

* Return error

Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
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.

4 participants