feat: zniffer

[robertsLando]

[coveralls]

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%
<!--	Total:	4	837

Files with Coverage Reduction	New Missed Lines	%
api/lib/SocketEvents.ts	1	0.0%
<!--	Total:	1

Totals
Change from base Build 9272143414:	-0.6%
Covered Lines:	3899
Relevant Lines:	19590

---

💛 - Coveralls

[AlCalzone]

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:

I don't think generating keys there makes sense, as you want to spy on an existing network with existing keys: 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]

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

[AlCalzone]

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

[AlCalzone]

CAPTURE should be renamed so it's clear that it saves to a file.

[AlCalzone]

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

Speaking of the table, we probably want infinite scrolling, not pagination. 10 frames per page can easily be surpassed with a single communication attempt.

[robertsLando]

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

[AlCalzone]

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

Cloning might make sense to debug the current network.

Other than that, some observations (I know the layout is not optimized yet):

• [x] Starting the Zniffer is somewhat unreliable. I have to sometimes start/stop twice. With my test script it works fine.

• [x] Knowing whether the Zniffer is running or not (e.g. by using start/stop icons) would be nice

• [x] The "menu" bar should be sticky IMO:

• [x] The region column is not necessary in the table. We can only zniff one region at once and that is a configuration setting. It should probably be in the Zniffer UI though like in the Windows app.

• [x] Home ID should be formatted as HEX

• [x] Type is meant for the application to decide how to display a frame, not for the end user. We may want to color code the lines by frame type and have a legend for the colors.

• [x] Sequence No, TX Power should be in the frame details (which we don't have yet), as it only exists for some of them.

• [x] Source, destination and repeaters should probably be merged into a single column, that also highlights which hop the frame was sent through, similar to the log output [070 » 069 › 001]. if we do this, we need to still be able to filter by source and destination though.

• [ ] Protocol Data Rate takes up a lot of room. I'd probably use the icons we also use in the node list, and only display the speed as text.

• [x] The windows application displays the time that passed since the previous event in milliseconds. This is useful IMO.

• [x] Move the Zniffer tab all the way to the bottom. It's not useful for 99% of users.

• [x] If possible, less padding in the table. More room for data. No line breaks!

[AlCalzone]

Additional findings in the latest version:

• [x] Timestamp should have fractions of seconds HH:mm:ss.fff. Not sure about the date - it seems that it takes away unnecessary room in the table.
• [x] Wrong enum for protocol data rate, should be ZnifferProtocolDataRate
• [x] When enabling "convert RSSI" in the settings, the RSSI column should show ${rssi} dBm, not ${rssiRaw}
• [x] Delta[ms] makes more sense directly after the timestamp column
• [x] I'd put Home ID before the source/dest columns
• [x] Regarding the details pane:
• [x] Nested CCs aren't shown - this should contain an IndicatorSet:
• [x] Can't reproduce it now, but some toLogEntry results are multiline, the details view currently ignores \n

[AlCalzone]

More things:

• [x] Routed frames seem to be shown as buffers, not CCs
• [x] The selected row should be highlighted
• [x] When selecting a row, pause autoscroll. Re-enable when scrolling all the way down (or using a button in the UI)

[AlCalzone]

• [x] Dragging results in weird layout jumps: Screencast from 14.05.2024 12:12:06.webm

[AlCalzone]

• [x] Frequency selection should happen at runtime: https://zwave-js.github.io/node-zwave-js/#/api/zniffer?id=frequency-selection
• [x] We need documentation somewhere to explain how the filters must be written. I didn't realize the homeID had to be 0x prefixed in the filter bar.

[AlCalzone]

• [ ] Save Capture should download the file, like NVM backup does
• [x] Do not resubscribe event handlers when stopping and starting

[kpine]

1. Typo:

2. Once you enter an invalid search query, the error message is never cleared and the tooltip never returns. Only option is to leave the window and come back.

3. If you leave the Zniffer window (e.g. go to settings) 1) the existing trace on screen is lost completely and 2) no traffic is captured to the screen. Some possible options:

   • Open a modal dialog with a warning message for confirmation when leaving
   • Preserve trace and restore when returning, keep capturing in the background. Or restore some X percentage of past logs.
   • Allow opening the zniffer in a separate window, like the debug log

   My imagined use case for this would be to leave the zniffer running in the background unattended. Is this use case supported? I left the window and performed some activity, and when I came back the trace window was empty, but saving to file produced something, I just haven't checked the contents.

4. Is there a plan to add the ability to load a zlf file and display in the same UI?

~~5. Is there a future use case for this window resizing? Wondering why it just doesn't expand to the entire height of the window automatically.~~

[AlCalzone]

5. Is there a future use case for this window resizing? Wondering why it just doesn't expand to the entire height of the window automatically.

Click one of the captured frames :)

[kpine]

5. Is there a future use case for this window resizing? Wondering why it just doesn't expand to the entire height of the window automatically.

Click one of the captured frames :)

Yeah, I did that and still asked the question. :man_facepalming:

[robertsLando]

Thanks @kpine ! About the lost frames, I dunno if there is an api actually to fetch in memory frames from zniffer, @AlCalzone ?

I could store them on my side but I think the zniffer instance already have them somewhere in order to create the capture.

Also it's a nice idea to be able loading a capture file from UI, didn't thought about that. Cc @AlCalzone

About the rest I can take a look tomorrow 🙏🏻

[AlCalzone]

• [x] This tooltip doesn't make sense. You're now showing the current frequency, right? I'd just remove the text below the dropdown.

• [x] The RSSI unit is duplicated when having "convert RSSI" enabled:

• [x] Timestamps should be zero-padded to 3 digits:

[AlCalzone]

The layout bothers me a bit:

Also, is there any way to give the columns a fixed/min width to avoid this? Maybe even disable the sorting altogether?

[kpine]

Thanks @kpine ! About the lost frames, I dunno if there is an api actually to fetch in memory frames from zniffer, @AlCalzone ?

I could store them on my side but I think the zniffer instance already have them somewhere in order to create the capture.

Not critical functionality in my mind. The warning when leaving via navigation, or an "open in new window" are both good solutions. I can of course open my own new window to avoid the problem. The ease of losing the current trace with a miss-click is the main problem.

[AlCalzone]

Yep I should really just expose the captured frames so Z-UI can restore the state from those, and add a hook to actually clear them.

EDIT: Next release will include that ->> https://github.com/zwave-js/node-zwave-js/pull/6852

[AlCalzone]

I feel like this is going into nitpick territory, but number columns should be right-aligned:

[AlCalzone]

Regarding formatting of the route - I'd also put that in the overview table, so you can see the route there immediately. Routed frames have a hop property that tells us where along the route we are. The direction property tells us in which direction along the route we are going and does not have to be listed separately.

For reference, this is how this frame's route is formatted in the logs:

[AlCalzone]

• [x] If I don't set a default frequency in the settings, the current frequency is not shown here:

• [x] I should not be able to clear the current frequency in the frontend, only the default in the settings

[AlCalzone]

• [ ] Wound be nice to see if the Zniffer is running without being on the tab, for example:

[robertsLando]

TODOs:

• [x] Show if zniffer is running on tab https://github.com/zwave-js/zwave-js-ui/pull/3706#issuecomment-2111844512
• [ ] Save capture should download the file not storing it on store (or both?)
• [x] Protocol Data Rate takes up a lot of room. I'd probably use the icons we also use in the node list, and only display the speed as text. cc @AlCalzone ?
• [ ] Understand if removing security keys from settings and allow to change them on fly (needs driver support)
• [x] Don't allow driver and zniffer to use the same port (when both are enabled)

[AlCalzone]

• [x] Raw payload in details view is taking up too much room. One or two lines should be enough. If the payload is very long, it can expand automatically.
• [x] Raw payload should have a label (e.g. Raw:) in front of it
• [x] Probably move the raw payload below the parsed payload?
• [ ] Probably omit the 0x from buffers
• [x] All buffers should be displayed in a monospace font (raw payload, CC payload)
• [x] The resize bar should have a slightly gray background IMO
• [x] Use the new parameter of znifferProtocolDataRateToString to omit the protocol name (https://github.com/zwave-js/node-zwave-js/pull/6863), use icons to distinguish Z-Wave and LR
• [x] trimStart on log entry details:
• [x] Add line number as first column
• [x] Show somewhere how many frames have been captured
• [x] Indicate "speed modified" in the Protocol Data Rate column, e.g. with a 🐌 icon to the right of the speed:
• [x] Choose a default height for the details pane that is high enough for the entire table to be visible:
• [x] Add a button to resume auto-scrolling
• [x] Probably display something in the details pane when nothing is selected:
• [x] Add tooltips on hover to the action buttons:
• [x] Probably change X to a trashcan icon 🗑️
• [x] Add a pop-out button for the Zniffer UI, similar to the Debug logs

[AlCalzone]

• [x] Add a button to resume auto-scrolling

Am I blind? :)

[robertsLando]

@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?