zwave-js / zwave-js-ui

Full featured Z-Wave Control Panel UI and MQTT gateway. Built using Nodejs, and Vue/Vuetify
https://zwave-js.github.io/zwave-js-ui
MIT License
984 stars 201 forks source link

Displaying the failedPingsController attribute #3557

Open duylong opened 10 months ago

duylong commented 10 months ago

Hi,

Is the failedPingsNode attribute displayed all the time even when it's 0 ? I ran 2 tests on the same node and once it appeared, another time it didn't.

I don't have a custom route, and I haven't done any reconstruction in the meantime. Can you explain to me ?

102-126

102

102-2

126

robertsLando commented 9 months ago

I don't show them when they are undefined:

https://github.com/zwave-js/zwave-js-ui/blob/04f1c76e6305c88a745f0a039c6e1e3d10207327/src/components/dialogs/DialogHealthCheck.vue#L289-L317

I dunno sincerly why sometimes they are undefined and sometimes them are not, maybe that's an issue on driver not setting them to 0 by default. cc @AlCalzone

duylong commented 9 months ago

Wouldn't it be more relevant to display it anyway even if it's "undefined"? We have the impression that we have 0 ping losses to the controller when in reality the information is unknown.

I'm in the process of improving my network and I don't feel like I've ever seen a 0/10, I wonder if it really exists, do you have it on your side or is it just mine?

robertsLando commented 9 months ago

@duylong I have a feel that when missing it's 0/10, it's just the driver maybe that doens't set it. Let me talk with @AlCalzone to see what's the best solution

AlCalzone commented 9 months ago

Please make a driver log of a health check where they are missing, loglevel debug and attach it here as a file (drag & drop into the text field).

duylong commented 9 months ago

zwavejs_2024-01-30.zip

I ran a Health check for debugging. OK for Controller to Node, but no trace from Node to Controller.

AlCalzone commented 9 months ago

Ok, so this requires a bit of explanation: Measuring the failed pings from node -> controller requires that the node supports Powerlevel CC. If it doesn't, failedPingsController will be undefined. This seems to be the case for node 8 in the log.

When it is supported, the min. powerlevel without errors indicates the lowest powerlevel at which there were no failed pings from node -> controller. We could additionally display 0 failed pings here, but this feels redundant to me.

Currently, the only situation when failedPingsController is shown, is when there was no reduction in powerlevel and there were still ping failures, as seen in the health check for node 102.

duylong commented 9 months ago

Not all that easy O:-)

I just did a test again, and indeed when minPowerlevel is 0, the failedPingsController field appears. In other cases, it is not defined.

  results: [
    {
      latency: 10,
      failedPingsNode: 0,
      numNeighbors: 23,
      routeChanges: 0,
      snrMargin: 8,
      rating: 7,
      minPowerlevel: 2
    },
    [length]: 1
  ],
  rating: 7,
  targetNodeId: 1
}

  results: [
    {
      latency: 60,
      failedPingsNode: 0,
      numNeighbors: 23,
      routeChanges: 0,
      snrMargin: 19,
      rating: 2,
      minPowerlevel: 0,
      failedPingsController: 2
    },
    [length]: 1
  ],
  rating: 2,
  targetNodeId: 1
}

  results: [
    {
      latency: 60,
      failedPingsNode: 0,
      numNeighbors: 23,
      routeChanges: 0,
      snrMargin: 19,
      rating: 7,
      minPowerlevel: 1
    },
    [length]: 1
  ],
  rating: 7,
  targetNodeId: 1
}

For me it would be necessary to continue displaying node -> controller to maintain display consistency, it's complicated to understand when sometimes it is displayed, sometimes not.

Additionally, is there a way to detect and display when powerlevel cc is not available? This way we are not surprised by the absence of the failedPingsController information.

AlCalzone commented 9 months ago

@robertsLando I think this is mostly a presentation issue. Maybe we should rework the columns to make this clearer:

  1. "Failed pings" --> "Failed pings 1 -> 102", only display failedPingsNode
  2. "Min. power level w/o errors" --> "Min. powerlevel 102 -> 1". Display failed pings and powerlevel together: a. "3 failed pings @ normal power" (if minPowerlevel is "normal power", the failed pings are relevant) a. "no failed pings @ -6 dBm" (only minPowerlevel is relevant here)
github-actions[bot] commented 6 months ago

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

github-actions[bot] commented 3 months ago

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

github-actions[bot] commented 3 weeks ago

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label