OPNSense widget

[Oupsman]

First working version of OPNsense widget. CPU and memory usage are not quite reliable yet.

[shamoon]

Cool. Why are cpu and memory unreliable? Regex matching in general isn’t the most reliable thing but sometimes unavoidable

[Oupsman]

Cool. Why are cpu and memory unreliable? Regex matching in general isn’t the most reliable thing but sometimes unavoidable

Well, CPU usage sometime displays "NaN" and I can't understand why. The memory usage displayed by the widget is not the same as in the dashboard. Values returned by the API call are not the same. I've asked why on the OPNsense forum. If I don't get any answer, I think I'll have to write a special proxy for OPNsense and call the widget API, which is not callable in API mode (you have to be logged on the firewall interface to call it).

EDIT : beside, I'm only using regex for isolate the uptime. To isolate the CPU and memory values, I'm using the split function. I can send you the JSON result of the API call if you want.

[shamoon]

Gotcha. In general we prefer widgets to not use their own proxy if possible, hugely reduces code maintenance, errors, etc

Once I figure out how to get one of these setup I can test and review

[Oupsman]

Gotcha. In general we prefer widgets to not use their own proxy if possible, hugely reduces code maintenance, errors, etc

That's what I thought and honestly, I would really prefer it without a proxy.js file. I've found the reason why the CPU value is unreliable, my split code is wrong : sometime, values are separated by two spaces, and sometime, not. So I think I'll use substr instead.

EDIT : it seems the substr function is deprecated so I'm using substring instead.

[shamoon]

Overall nice, works great.

I think the calculations are overkill, I would just change to using the statistics reported from the api.

I also think that 5 blocks gets pretty cramped (see below) so I would advocate for reducing this by at least 1. In fact I would say to remove the WAN data and then you can remove the second API call to keep things speedier. But up to you on that last point, I didnt include it in the suggestions.

[shamoon]

couple other random things

[Oupsman]

I also think that 5 blocks gets pretty cramped (see below) so I would advocate for reducing this by at least 1. In fact I would say to remove the WAN data and then you can remove the second API call to keep things speedier. But up to you on that last point, I didnt include it in the suggestions.

That is the data requested in the enhancement request.

But I'm wondering if the uptime is really that important a data to see : if the firewall were to reboot, the WAN datas will be reset to zero at the reboot moment so you'll see a huge decrease on the widget.

[shamoon]

But I'm wondering if the uptime is really that important a data to see : if the firewall were to reboot, the WAN datas will be reset to zero at the reboot moment so you'll see a huge decrease on the widget.

Yea but as the widget author it’s really up to you :)

[shamoon]

Mate, you asked for feedback and are ignoring it even though you mentioned not being very comfortable with JS. We appreciate your contribution but please understand that part of the job here is to try and maintain consistent code quality.

The memory calc isnt working in my testing.

Again, my suggestions are meant to simplify your code and make it less error-prone. Please adjust.

[Oupsman]

Mate, you asked for feedback and are ignoring it even though you mentioned not being very comfortable with JS. We appreciate your contribution but please understand that part of the job here is to try and maintain consistent code quality.

The memory calc isnt working in my testing.

Again, my suggestions are meant to simplify your code and make it less error-prone. Please adjust.

I am trying the regex to get the idle value right now. Sorry if you thought I'm not taking your advice into account, I'm trying to keep up with you, on the contrary.

[shamoon]

It was a code suggestion above, you can just apply suggestions using the GH site

[shamoon]

Let me know if youre still working on this (obviously if youre offline no worries)

[Oupsman]

Let me know if youre still working on this (obviously if youre offline no worries)

I have to work (holidays are over) but I think I'll work on it in the evening. As the memory code is really unreliable (as per your testings, I'm not the only one in this case) I think I'm going to drop it, because I think it's better to report no value than a unreliable value.

[shamoon]

Let me know if youre still working on this (obviously if youre offline no worries)

I have to work (holidays are over) but I think I'll work on it in the evening. As the memory code is really unreliable (as per your testings, I'm not the only one in this case) I think I'm going to drop it, because I think it's better to report no value than a unreliable value.

I agree. That change is also a suggestion here, if you dont mind we can apply and get this merged and done.

[Oupsman]

Let me know if youre still working on this (obviously if youre offline no worries)

I have to work (holidays are over) but I think I'll work on it in the evening. As the memory code is really unreliable (as per your testings, I'm not the only one in this case) I think I'm going to drop it, because I think it's better to report no value than a unreliable value.

I agree. That change is also a suggestion here, if you dont mind we can apply and get this merged and done.

If I understand correctly, your proposition is to report the active memory, and I don't think that's a meaningful value. I'm more into purely stopping to report memory usage until I have a reliable way to retrieve it.

[shamoon]

If I understand correctly, your proposition is to report the active memory, and I don't think that's a meaningful value. I'm more into purely stopping to report memory usage until I have a reliable way to retrieve it.

I mean, if you feel strongly OK, but I would think most people would find the active memory value more useful than nothing, no?

[Oupsman]

If I understand correctly, your proposition is to report the active memory, and I don't think that's a meaningful value. I'm more into purely stopping to report memory usage until I have a reliable way to retrieve it.

I mean, if you feel strongly OK, but I would think most people would find the active memory value more useful than nothing, no?

That's a valid point, I've validated your commits.

[shamoon]

Cheers, thanks for all the effort

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion for related concerns.