travisghansen / hass-opnsense

OPNsense integration with Home Assistant
194 stars 25 forks source link

Allow interface traffic to be used in statistics #85

Closed Happyrobot33 closed 5 months ago

Happyrobot33 commented 1 year ago

Was lacking the utility sensors I had from when I was using a off the shelf router and integration, so I decided to add them :) I also fixed the fact that the display name of the interface sensors doesn't clean up the _ making them not look good if you directly add them to lovelace without overwriting the display name on a card

closes #84

Happyrobot33 commented 1 year ago

Needs fixing, the meters don't seem to reset for some reason. I'm likely reading the documentation wrong

Happyrobot33 commented 1 year ago

implemented a possible fix, give me a day to see if it works

Happyrobot33 commented 1 year ago

So I just realized I'm doing this wrong because last_reset is only happening on the sensor creation, not in update. But even reloading the integration doesn't seem to make the value drop to 0, so I really don't know what's wrong

Happyrobot33 commented 1 year ago

yeah this is more of a pain than I thought it would be, there doesnt seem to be anything a integration can hook into that just does this for them, so itd have to be coded manually to do the difference.....

Happyrobot33 commented 1 year ago

gonna try to see if I can do it through opnsense's api instead of emulating it in HA

Happyrobot33 commented 1 year ago

ok so TLDR, im stupid and didnt realize how home assistant statistic sensors worked. all that needed to be changed is for the state_class of some entitys to be total_increasing so home assistant knows how to store them

travisghansen commented 1 year ago

Thanks! I know it’s a pain but the _ business has been discussed ad-nauseam and I’d prefer not mess with that right now :(

The metrics look great! I think we may need to tighten the logic a little bit to ensure it only includes that attribute/value on appropriate sensors. Some of those sensors in that loop may be packets/bytes per second which would not correlate properly to the total increasing property.

alexdelprete commented 1 year ago

For cumulative sensors:

For instant measurement sensors:

(reference: https://developers.home-assistant.io/docs/core/entity/sensor)

Happyrobot33 commented 1 year ago

Thanks! I know it’s a pain but the _ business has been discussed ad-nauseam and I’d prefer not mess with that right now :(

The metrics look great! I think we may need to tighten the logic a little bit to ensure it only includes that attribute/value on appropriate sensors. Some of those sensors in that loop may be packets/bytes per second which would not correlate properly to the total increasing property.

Possibly make the _ configurable then?

The per second ones are already covered due to the order of the if statements, unless im missing something

travisghansen commented 1 year ago

Keeping configuration values at a minimum.

Indeed you are right we should be good for the per second stuff. I'll merge when I get a moment and snap a new release!

Happyrobot33 commented 1 year ago

Keeping configuration values at a minimum.

Indeed you are right we should be good for the per second stuff. I'll merge when I get a moment and snap a new release!

well hold on ill have to remove the _ change correct?

Happyrobot33 commented 1 year ago

ive removed the _ change so it should be good to merge @travisghansen

travisghansen commented 5 months ago

Thanks for the contribution! This is already available in current releases so closing.