Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

Automatic interview process for new nodes #4

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

stamp
Copy link
Contributor

@stamp stamp commented Jul 6, 2017

No description provided.

stamp added 2 commits July 6, 2017 23:02
Sometimes when you reconnect to a usb connected controller there are
data bufferd that are incomplete. By emptying it first we start on a
blank slate :)
Added NIF to the interview process and automatic restart of the
interview process if it isnt completed yet
@stamp stamp requested a review from lukescott July 6, 2017 21:13
empty:
for {
select {
case <-parserInput:
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of relying on go frameLayer.bgRead() to empty the buffer, could we perhaps consume what is on transportLayer instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the issue is to make something that is non-blocking and that was the best solution I could think of.. because when the buffer is empty the read will block until new data is sent from the controller.

Copy link
Contributor

@lukescott lukescott Jul 6, 2017

Choose a reason for hiding this comment

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

It looks like the reader may be backed by bufio.Reader. Could we expand the interface and add the Reset method? (Or something similar). Would it be possible to make/reset the buffer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea! Will take a look at that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #10

n.saveToDb()
n.nextQueryStage()
Copy link
Contributor

@lukescott lukescott Jul 6, 2017

Choose a reason for hiding this comment

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

Nothing jumps out at me here code-wise in this file. Not sure what this does well enough :). @bjyoungblood might have a better idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nextQueryStage is the method that runs each step of the interview process.

Right now the interview process is 4 steps

  • get the NIF
  • get security cababilities
  • get manufacturer info
  • get command classes versions

@codecov
Copy link

codecov bot commented Jul 12, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@d206e6b). Click here to learn what that means.
The diff coverage is 10.41%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master       #4   +/-   ##
=========================================
  Coverage          ?   20.02%           
=========================================
  Files             ?      371           
  Lines             ?    14743           
  Branches          ?        0           
=========================================
  Hits              ?     2952           
  Misses            ?    11768           
  Partials          ?       23
Impacted Files Coverage Δ
gen/bindata.go 0% <ø> (ø)
cmd/gozw-prompt/test.go 2.58% <ø> (ø)
cc/security/scheme-report.gen.go 21.87% <0%> (ø)
...c/sensor-multilevel-v10/supported-get-scale.gen.go 21.87% <0%> (ø)
cc/version/command-class-get.gen.go 21.87% <0%> (ø)
cc/notification-v4/event-supported-report.gen.go 16.27% <0%> (ø)
cc/basic-v2/report.gen.go 16.66% <0%> (ø)
cc/battery/report.gen.go 21.87% <0%> (ø)
cc/thermostat-setpoint/get.gen.go 82.35% <0%> (ø)
cc/configuration/set.gen.go 13.72% <0%> (ø)
... and 232 more

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 d206e6b...cf04d98. Read the comment docs.

@@ -106,6 +107,8 @@ func (s *Layer) receiveThread() {
return
}

fmt.Printf("Received: 0x%X\n", frameIn.Payload)
Copy link

Choose a reason for hiding this comment

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

Do we want some kind of leveled logging? logrus for example. This seems pretty low-level debug outout...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #9

@lukescott
Copy link
Contributor

@stamp, are all these commits based on the original PR?

@stamp
Copy link
Contributor Author

stamp commented Oct 2, 2017

Yes :)

The graph looks like this
screenshot from 2017-10-02 21-02-59

@lukescott
Copy link
Contributor

Sorry, my question might have been a bit vague. Just to make sure I'm not being confusing, I mean: Are they all related to "Automatic interview process"? Was just wondering if that feature is still on-going, or if I needed to do any additional review.

@stamp
Copy link
Contributor Author

stamp commented Oct 3, 2017

I think it works as it should. The issue here is that the interview process is not complete. There are more work to be done before the whole automatic interview process is complete but this code is ready to be used.

The open issue with frame/frame-layer.go: I did experiment with writing a Reset method but did not manage to get it to work.

I suggest we move both open discussions about frame/frame-layer.go and leveled logging to new issues instead and not solving them in this PR

asherry99 pushed a commit to asherry99/gozw that referenced this pull request Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants