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

feat: zniffer #3706

Merged
merged 124 commits into from
May 30, 2024
Merged

feat: zniffer #3706

merged 124 commits into from
May 30, 2024

Conversation

robertsLando
Copy link
Member

No description provided.

@robertsLando robertsLando marked this pull request as draft May 8, 2024 09:28
@coveralls
Copy link

coveralls commented May 8, 2024

Pull Request Test Coverage Report for Build 9298507831

Details

  • 4 of 837 (0.48%) changed or added relevant lines in 13 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.6%) to 21.166%

Changes Missing Coverage Covered Lines Changed/Added Lines %
api/lib/SocketManager.ts 0 1 0.0%
api/config/store.ts 0 2 0.0%
src/components/nodes-table/nodes-table.js 0 2 0.0%
api/lib/SocketEvents.ts 0 3 0.0%
src/lib/SocketEvents.js 0 7 0.0%
src/lib/items.js 0 10 0.0%
src/router/index.js 0 11 0.0%
src/stores/base.js 0 33 0.0%
api/lib/utils.ts 4 62 6.45%
api/app.ts 0 103 0.0%
Files with Coverage Reduction New Missed Lines %
api/lib/SocketEvents.ts 1 0.0%
Totals Coverage Status
Change from base Build 9272143414: -0.6%
Covered Lines: 3899
Relevant Lines: 19590

💛 - Coveralls

@AlCalzone
Copy link
Member

I'll just put my thoughts here...

The settings for Zniffer should probably not offer the serial port that's already in use by the controller:
grafik

I don't think generating keys there makes sense, as you want to spy on an existing network with existing keys:
grafik
We may also have to consider setting them on the fly to retroactively decode some commands, but that will require support in the driver first.

@AlCalzone
Copy link
Member

AlCalzone commented May 8, 2024

Filtering nodes makes sense in the table view but not in the settings IMO:
grafik

@AlCalzone

This comment was marked as resolved.

@AlCalzone

This comment was marked as resolved.

@AlCalzone

This comment was marked as resolved.

@robertsLando
Copy link
Member Author

The settings for Zniffer should probably not offer the serial port that's already in use by the controller:

Yeah but what if the driver is disabled? (still have to add this option)

I don't think generating keys there makes sense, as you want to spy on an existing network with existing keys:

Should I drop keys from there and use the one set on driver? Or add a button to clone the ones on driver?

We may also have to consider setting them on the fly to retroactively decode some commands, but that will require support in the driver first.

👍🏼

Saving settings for the Zniffer should probably not restart the driver for the controller.

Yeah agree

There's an error in the devtools and nothing is being shown in the table:

That error happens because I'm using ZWaveFrameType that seems not exported in safe: https://github.com/zwave-js/zwave-js-ui/pull/3706/files#diff-71a3cb674c70b3fa9b3f52e9afc7d873f7d0eeefcbccb7175f1448a816811a99

api/lib/ZwaveClient.ts Dismissed Show dismissed Hide dismissed
@AlCalzone

This comment was marked as resolved.

@AlCalzone

This comment was marked as resolved.

@AlCalzone
Copy link
Member

  • Add a button to resume auto-scrolling

Am I blind? :)

@robertsLando
Copy link
Member Author

@AlCalzone About the button I forgot to ask you what you mean because actually auto scroll is always enabled except if you have a frame opened in details, do you want to be able to re-enable it when you have a frame opened so?

@robertsLando robertsLando merged commit 18ffbe2 into master May 30, 2024
9 checks passed
@robertsLando robertsLando deleted the feat-zniffer branch May 30, 2024 07:14
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