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

admin.0 "Unsubscribe from all states, except system's..." #142

Closed
pdbjjens opened this issue Mar 14, 2021 · 24 comments
Closed

admin.0 "Unsubscribe from all states, except system's..." #142

pdbjjens opened this issue Mar 14, 2021 · 24 comments
Assignees

Comments

@pdbjjens
Copy link
Contributor

Describe the bug
If I choose all four options (L1, L2, L3, Extended Details) the iobroker admin reports: "Unsubscribe from all states, except system's, because over 3 seconds the number of events is over 200 (in last second 0)" when I have the Objects-Tab open. The update of the values in the objects-tab stalls in this case indefinitely long. If I don't have the Objects-Tab open, it does not unsubscribe and even re-subscribes all states automatically. If I choose no options or only extended Details, the admin does not unsubscribe states. Probably this is due to the fact that I have two Devices (SHM and SEM) and that they update all values by the second.

To Reproduce
Steps to reproduce the behavior:
See bug description

Expected behavior
The update rate of the values in the Objects tab should be throttled so that this behaviour can be avoided.
This could be implemented by a configurable debounce time between 1 and 60 sec to reduce system load
(by reducing the setstate load).
The debounce time should be configurable via the Config page. It should allow also the option to have no debounce
since in some cases the raw values are needed in realtime (i.e. update once per second as the datagrams arrive).
Some of the values fluctuate very much (e.g. the power values or frequency).
For power values the debounce values should be computed over the debounce time as a
mean value (=artithmentic average), for frequency median might be better.
For values which must not be averaged (like f.i. the Energy counters like pregardcounter),
only the last value in the last datagram of the debounce period should be written (setstate).
The debounce interval should be aligned with multiples of the datagram repetition time to avoid jitter.
F.i. if the required debounce time is 30 sec, then the average should span over 30 datagrams
(provided the datagram repetition time is 1 sec).

Screenshots & Logfiles
iobroker.2021-02-07.log.zip

Versions:

  • Adapter version: <0.6.3>
  • JS-Controller version: <3.1.6>
  • Node version: <v12.19.0>
  • Operating system: <Windows 10>

Additional context
None

@TGuybrush
Copy link

A new diagram is send every second from the SMA unit.
I propse to introduce two configuration values to influence the update rate in seconds starting with 1s as minimum and default value.

  • Group 1: For realtime values
  • Group 2: For the counters and meta information like the software version.

@pdbjjens
Copy link
Contributor Author

@TGuybrush
I am really glad that you agree to work on this issue.
I think that having 2 separate debounce times for realtime and nonrealtime values is a good proposal to further
reduce the setstate rate but still keep a good realtime response.

Please keep in mind during implementation that the realtime values should be averaged (mean value = arithmetic average) in memory over the debounce period for power values before they are written to the database (setstate).
For other realtime values like frequency or cosphi "median" might be better.
For non-realtime states, only the last value in the last datagram of the debounce period should be written (setstate).
Thus for each of the states the averaging algorithm should be assignable separately.
A possible implementation might be a mapping table which states should be debounced how.
Although the update rate(s) (=debounce time) could be configured by the user in seconds, I think that internally the adapter
should be triggered by the arriving datagrams, not by a timer. (see above: F.i. if the required debounce
time is 30 sec, then the average should span over 30 datagrams (provided the datagram repetition time is 1 sec). Otherwise the averaging algorithms might produce wrong results because of the jitter between the timer and the datagrams.

@DutchmanNL
Copy link
Member

I think that having 2 separate debounce times for realtime and nonrealtime values is a good proposal to further
reduce the setstate rate but still keep a good realtime response.

fully agree as some values are really needed (think about using SMA for load balancing etc than live is a must) and others are fine to just have an interval x time on it to refresh like every 10 (or xxx) seconds

A possible implementation might be a mapping table which states should be debounced how.

My proposal here would be to have an additional JSON library implemented with additional config parameters.
In generic all my adapter use a stateAttr.json library were this kind of mappings kan be made.

This will keep the code static and flexibel, but would allow easy modification for this specific part.
Maybe we should work with classes. to implemented that, not sure....

Although the update rate(s) (=debounce time) could be configured by the user in seconds, I think that internally the adapter
should be triggered by the arriving datagrams, not by a timer. (see above: F.i. if the required debounce
time is 30 sec, then the average should span over 30 datagrams (provided the datagram repetition time is 1 sec). Otherwise the averaging algorithms might produce wrong results because of the jitter between the timer and the datagrams.

agree, the adapter will receive events anyway and handle in memory which has. no effect on system load or state/object library.
It makes more sense than to have an internal "memory" debounce instead of a time picking the values at x moment of time

@pdbjjens
Copy link
Contributor Author

@TGuybrush
While you are working on this issue #142, please also correct the new issue #152

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Apr 1, 2021

@TGuybrush
We got another incident of issue #137 with some details. Also the originator of #137 sent some more details in the forum warnungen-sma-em-adapter Currently I do not have any idea what happens there. Do you?

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Apr 2, 2021

@TGuybrush
Meanwhile the reason for issue #137 seems to be identified. The warnings orinate from the fact that the adapter interprets multicast response messages from the SMA Device Discovery Protocol as Energy Meter datagrams and thus wrongly sets up fake devices in the object tree. The adapter should filter out the Device Discovery datagrams and not set up any datapoints for that. This could be done by looking at the protocol-ID (byte offset 17&18) in the multicast datagram header which is 0x6069 for the SMA-EM protocol and 0x0001 for the Device Discovery protocol. Would you please build that into the code of the upcoming new version?

@TGuybrush
Copy link

Thx I think you're right. I will add a better detection of the protocol in the UDP packet.
I will provide an updated version.

@TGuybrush
Copy link

I've pushed a new version in my repository which adds a verification of the packet header. Please test it in your environment.

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Apr 8, 2021

@TGuybrush
The reporters of #137 have successfully tested your fix in their environment. See discussion in
[https://forum.iobroker.net/topic/42269/warnungen-sma-em-adapter]https://forum.iobroker.net/topic/42269/warnungen-sma-em-adapter). Please include this fix in the next PR.
Please also look into issue #152 . Since these warnings appear with all new js-controller versions, I think this should also be fixed before we release the next Beta (0.6.4 ?)

@pdbjjens
Copy link
Contributor Author

@TGuybrush
Please note that we recently released v0.6.4, which includes also some other fixes and PRs besides yours. So make sure to merge the current master to your branch before you start working on issue #142. When do you think you can provide the PR containing this new feature? Before we bring this adapter to the stable repo, I would love to include the configurable debounce time because it makes the adapter much more stable and reliable in general use cases.

@pdbjjens
Copy link
Contributor Author

@TGuybrush
Could you have a look into issue #348?
I habe already tried to fix this by adding client.close() to the onUnload method, but to no avail.

@pdbjjens pdbjjens reopened this Feb 18, 2022
@pdbjjens
Copy link
Contributor Author

@TGuybrush
FYI: I fixed issue #348 and released it with v0.6.5. So no need for you to look into this issue anymore.
BTW: do still intend to work on this issue #142 ? If not, please let me know, if yes, please inform me by when you think to provide a PR.

@TGuybrush
Copy link

I'm currently working on it. I think it should be done within the next two weeks.

@pdbjjens
Copy link
Contributor Author

@TGuybrush @DutchmanNL
Great!, I am looking forward to your PR then.
Don't hesitate to ask us (me and DutchmanNL) if you have any questions regarding the expected behaviour which we specified above. Btw. we have a Telegram chat ongoing regarding the sma-em adapter. Are you interested in joining? It would make communication easier. Just let me know.

@DutchmanNL
Copy link
Member

we have a Telegram chat

https://t.me/+I0TJdMKCOaBLd2ZW

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Jun 6, 2022

@TGuybrush
There is a new issue #403. Do you have any idea what could be wrong there? I thought that the adapter binds to all available interfaces simultaneously. More info is avaible in the forum [https://forum.iobroker.net/topic/43011/test-adapter-sma-em-v0-6-x-latest/171](url)

@pdbjjens
Copy link
Contributor Author

@TGuybrush
do still intend to work on this issue #142 ? If not, please let me know, if yes, please inform me by when you think to provide a PR.

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Mar 1, 2023

@TGuybrush
Since I did not hear from you for more than a year now, I wonder whether you still want to work on this issue #142 ?
If not, please let me know, so that we can find somebody else who can spare some time to implement this.
If yes, please note that I just released the 2023 maintenance release v0.6.6

@pdbjjens
Copy link
Contributor Author

pdbjjens commented Mar 7, 2023

@TGuybrush @DutchmanNL
FYI: Since I could spare some time, I have started working on this issue #142 myself. Hopefully I will be able to provide a PR soon.

@pdbjjens pdbjjens self-assigned this Mar 7, 2023
@pdbjjens
Copy link
Contributor Author

pdbjjens commented Mar 8, 2023

@DutchmanNL @TGuybrush
I have finished a first shot of the PR. Before I do the PR I would like you to do a brief review of the code and have a look at whether this fits your expectations. I have also changed to the json config and improved language support a little (at least en and de now).
Please see: https://github.com/pdbjjens/ioBroker.sma-em.git

@pdbjjens
Copy link
Contributor Author

@DutchmanNL @TGuybrush
I have issued PR #529 Would you like to review it before I merge and release it as v0.7.0 ?

pdbjjens added a commit that referenced this issue Mar 14, 2023
Fixes issue #142 admin.0 "Unsubscribe from all states, except system's..."
@ticaki
Copy link
Contributor

ticaki commented Aug 21, 2023

@pdbjjens
Zu der Ereignissmenge: Ich hab aktuell noch keinen Bedarf(abgesehen von besserer Anzeige) für hohe Aktualisierungsraten. Jedoch auf Grund der unterschiedlichen Hostsystemen auf denen ioBroker läuft bin ich der Meinung das es dem Nutzer überlassen sein sollte.

@pdbjjens sagte in Test Adapter sma-em v1.0.x Latest:

Zum Vergleich mal die Ereignisrate der vorigen Version (0.6.6) d.h. ohne Throttling:

Mit DetailsL1-3, Erweitert, 1s/30s und 1 SHM 200ms + 1 EM 1s 90/5666 Ereignisse.
Hierbei tritt dann auch der Fehler auf: Unsubscribe from all states, except system's, because over 3 seconds the number of events is over 200 (in last second 0) und im Objects Tab werden die States nicht mehr upgedatet.
Mit DetailsL1-2, Erweitert, 1s/30s und 1 SHM 200ms + 1 EM 1s 90/4316 Ereignisse.
Hierbei tritt dann auch der Fehler auf
Mit DetailsL1, Erweitert, 1s/30s und 1 SHM 200ms + 1 EM 1s 90/2966 Ereignisse.
Dies geht gerade noch ohne Fehler.
Ohne Details, Erweitert, 1s/30s und 1 SHM 200ms + 1 EM 1s 90/1616 Ereignisse.
Kein Fehler.
Die Grenze liegt also bei etwa 3000 Ereignissen.
(Zumindest auf meiner Test-HW,
Plattform: Windows
Betriebssystem: win32
Architektur: ia32
CPUs: 2
Geschwindigkeit: 2394 MHz
Modell: Intel(R) Core(TM)2 Duo CPU T7700 @ 2.40GHz
RAM: 3.2 GB
Node.js: v18.16.0
Adapter-Anzahl: 580
NPM: 9.5.1
Aktive Instanzen: 3

Ohne Details, erweitert, 1s/30s SHM 2.0 182 Ereignisse. In meinem Fall wäre ich nun froh wenn es mir möglich wäre die Aktualisierungsrate zu erhöhen und ich würde es auch nutzen. Hätte dann 900 Ereignisse was noch immer die Hälfte des Shelly Adapters wäre.

apollon hat mir diese Info gegeben: https://www.iobroker.net/#de/blog/2021_12_15 das sind setState() pro Sekunde
und es gibt scheinbar keine feste Grenze. Im Javascript Adapter kann man diese auch beliegig einstellen. Das zugrunde liegende System muß aber natürlich fix genug sein.

Ist aber natürlich dir überlassen, du wirst nachher bugreports von Leuten mit Raspi1 bekommen. :)

@pdbjjens pdbjjens reopened this Aug 21, 2023
@pdbjjens
Copy link
Contributor Author

@ticaki
Ich habe die Diskussion mit Interesse mitgelesen. Dabei ist mir der grundsätzliche Tenor aufgefallen, der sich auch mit meiner Einstellung deckt, dass man mit ioBroker keine Realzeitsteuerung aufbauen kann und auch aus Sicherheitsgründen nicht zulassen sollte.
Bezüglich der bugreports von Raspi 1 usern mache ich mir keine Sorgen, Wenn die bisher mit dem Adapter auskommen konnten, werden sie jetzt erst recht keine Probleme haben. Im Gegenteil, wenn ich die Performancewerte so anschaue sollte ich den Default-Wert für die Realzeit-Intervalle noch höher setzen ;-)
Um Deine Frage bezüglich der Erhöhung der Aktualisierungsrate zu beantworten: In der aktuellen v1.0.0 ist die Mittelwertbildung für die Realzeitwerte abschaltbar, d.h. die Updaterate entspricht dann der des sendenden SHM/EM.
Die Abschaltung erfolgt durch Einstellung eines Realzeit-Intervalls von 0.
Du hast sicher Verständnis, dass ich dieses aus o.g. Gründen nicht an die große Glocke hängen möchte und Dich bitten diese Information für Dich zu behalten.

@ticaki
Copy link
Contributor

ticaki commented Aug 21, 2023

Tenor

Was aber daran liegt, das sie keine Anwendungsmöglichkeit sehen. Bevor ich mir die Batterie für 7000€ bestellt habe, hatte ich ein Skript das sie simuliert - mit 200ms wäre das erheblich genauer gewesen. Realtimesteuerung halte ich für kein sinnvolles Usecase da müsste man schon auf 10ms kommen.

Das mit den Performancewerten verstehe ich so nicht wirklich für folgenden Code (Javascript-Adapter) braucht mein Rechner zwischen 3,2 und 4,6 Sekunden

let start = new Date().getTime()
let a = 0 
async function test() {
    while (a++<3000) await setStateAsync('0_userdata.0.Test', a*a, true)
    log('zeit: ' + (new Date().getTime() - start))
}
test()

Nuc Intel N100 16gb ssd
Proxmox VM 2 Cores maximal 170% Auslastung erlaubt.

nicht an die Glocke hängen

Hab ich Verständnis für, das sollte man nicht einfach nur weil man es kann verwenden. Ist ja in 99% der Fälle nur Ressourcen Verschwendung.

EDIT:

Script script.js.Test.Skript_122 is calling setState more than 1000 times per minute! Stopping Script now! Please check your script!

Ja 3000 sind mehr als 1000 :) Die Zahl ist aber für den Adapter konfigurierbar.

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

4 participants