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

Bugfix: stop polling every five seconds #147

Closed
dirkjanfaber opened this issue Sep 30, 2022 · 11 comments
Closed

Bugfix: stop polling every five seconds #147

dirkjanfaber opened this issue Sep 30, 2022 · 11 comments
Assignees

Comments

@dirkjanfaber
Copy link
Collaborator

Currently the nodes poll every five seconds for information. Even when the information remains the same, it is pushed through the flow again. This should be changed by pushing it only once on startup and on change.

@dirkjanfaber dirkjanfaber self-assigned this Sep 30, 2022
@dirkjanfaber
Copy link
Collaborator Author

It appears that this can also be seen as a feature, so disabling this might disappoint some users. Perhaps the better alternative is to start using the config node and making it configurable. And consider sending a cached answer instead of poling again.

@pdcurtis
Copy link

pdcurtis commented Nov 8, 2022

@dirkjanfaber I certainly use this as a feature, for example I use it for smoothing, to create daily totals and for limit checking. There are easy workarounds but it could make a lot of extra work and testing for some users with established systems. Caching would be sensible and invisible to users.
An option in nodes would a very useful enhancement, for example, I frequently reduce the flows using filter (RBE) nodes which would saved or limit the flows by suitably configured delay nodes. I monitor processor loads but do not have any accurate measurements on the overheads of various configurations of nodes and flows.

@mpvader
Copy link
Contributor

mpvader commented Nov 8, 2022

Hi, What is the node-red common practice for this?

For those flows that need repeat values, a node will exist already to take care of that I’d think?

@pdcurtis
Copy link

pdcurtis commented Nov 9, 2022

@mpvader I have used the combination of saving in Context using a Change node with an Inject Node when I need a steady stream of messages. I do not know of a single node to do it.

You are welcome to look at my code if you want - remote access is enabled and Site name is NB Corinna. But note:

  1. Since updating to 2.91 and 2.92 Load Current seems to have disappeared from the Solar Charger Node so quite a lot of relevant code is disabled temporarily until I investigate. Odd as it is still on Portal.
  2. I am testing the changes in @dirkjanfaber 's faberd/issue-144 branch (b4a1d83) which seems very successful in meeting its objectives.
  3. I have Persistent Global Context Storage enabled so beware if you copy any code!

@tkurki
Copy link
Contributor

tkurki commented Nov 9, 2022

So if I get only changes how do I tell everything is working? If I want to get notified about potential error condition of missing data how do I do that?

Afaik there is no canonical way to do this in NR, depends on how the data is produced. A switch closes and triggers an update via interrupt vs a temperature sensor is read periodically.

I’d make polling interval configurable and maybe add an option to send only changes, not change the default behavior. Nor would i classify this as a bugfix.

@dirkjanfaber
Copy link
Collaborator Author

So if I get only changes how do I tell everything is working?

With the status node, one can check if the node still is connected, which indicates that everything is still working.

That being said, I understand the concerns and benefits about getting data every x seconds. Fetching the answer from the cache instead of from the dbus will decrease the bus load, which should be the default. Basically re-sending the last answer periodically.

Getting it in a configuration pane is what I aim for. Perhaps even with the non-default choice of making it possible to enable fetching fresh data every x seconds from the dbus instead of from the cache.

@mpvader
Copy link
Contributor

mpvader commented Nov 9, 2022

Hey Dirk-Jan, if everybody prefers it the way it is now, resending every five seconds, with added benefit that 100% there is nothing going wrong thats related to a cache (and caches often cause issues...), simply by not having a cache: then for me its also fine to just leave it all as is now.

The bit of extra D-Bus load is acceptable to me.

@pdcurtis
Copy link

I have just noticed that the polling interval is no longer exactly 5 seconds but faster and the timing is no longer regular see screenshot of debug window of output from a SmartSolar MPPT 100/20 48
Screenshot from 2022-11-10 11-35-16

@thepaulcooper
Copy link

I vote for keeping the 5s poll or, if making it configurable, keep it as the default.

dirkjanfaber added a commit that referenced this issue Feb 1, 2023
This makes an input node only send out changes if the 'onlyChanges'
checkbox is checked. As it acts on the dbus ItemsChanged property,
an initial value will not be send.
@dirkjanfaber
Copy link
Collaborator Author

The solution will be an extra checkbox on the edit panel:
image

When checked, the node will output only on value changes. When not checked, the behaviour is the same as it has always been.

@dirkjanfaber
Copy link
Collaborator Author

Closing this issue (merged with master)

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

No branches or pull requests

5 participants