fredlcore / BSB-LAN

LAN/WiFi interface for Boiler-System-Bus (BSB) and Local Process Bus (LPB) and Punkt-zu-Punkt Schnittstelle (PPS) with a Siemens® controller used by Elco®, Brötje® and similar heating systems
231 stars 84 forks source link

Optional logging via UDP broadcast added (configurable, same parameters and format as in SD card logging) #471

Closed DE-cr closed 2 years ago

DE-cr commented 2 years ago

c.f. https://github.com/fredlcore/BSB-LAN/issues/432

tested:

not tested:

DE-cr commented 2 years ago

Github says "All checks have failed". Hmm, the code compiles perfectly in my Arduino 1.8.19 IDE.

Could it be that github requires preprocessor defines to start in the first column? I placed them further to the right, for improved readability. I guess I'll move them to the very left and try again...

DE-cr commented 2 years ago

Problems with github checks fixed:

fredlcore commented 2 years ago

Thanks for the PR, please find my few comments in the code.

DE-cr commented 2 years ago

You may want to check out the following:

  1. Does my edit in BSB_LAN.ino line 5689 follow that code section's intent?
  2. Are you offended by my using "if (condition) short_code_line;" instead of "if (condition) {\n short_code_line;\n}"?
  3. Is the setPPS() function (which I did not touch) ok for you?: 3.1. It seems to be unused. 3.2. Its header comment says "Global resources used: pps_values[]", but its code also uses (reads) the following global variables: LoggingMode, numLogValues, log_parameters[].
fredlcore commented 2 years ago
  1. No it doesn't, but previous sections of the code don't either, so it's a bit hard to do preprocessor indention correctly.
  2. Personally, I prefer the second way even for single lines of code because with the first way, you don't see at first glance if there is an indention mistake. But various contributors have used this way before, so I don't mind.
  3. It is used in pps_handling.h, but you are right, the function comments are updated poorly.
DE-cr commented 2 years ago

Re 1.: I meant intent, not indentation: Does my code change do what the code should do? The original version of that line seemed to be wrong to me. Re 3.: A common problem. Personally, I prefer no comments to misleading ones. ;)

fredlcore commented 2 years ago

Re 1: Right, I was wondering about that, too. Could you have a look at that, please, @dukess?

DE-cr commented 2 years ago

This may be worth mentioning: In my testing of the UDP broadcasts, every now and then some packets got lost in transit (server=BSB_LAN to client=Perl@Linux). As far as I'm concerned, that's ok/expected: This is UDP broadcasting, not TCP data exchange with handshake.

However, I did NOT notice lost packets with the custom code for UDP broadcasting that I've been using for my https://github.com/DE-cr/BSBmonCR

The difference in the implementation is:

(Six parameters to be logged/sent was my test set.)

fredlcore commented 2 years ago

Thanks, also for the observation regarding dropped packages. Yes, we can't expect UDP to be reliable, but it would be great to figure out if this is a general behaviour now or whether it does not exist with fewer parameter (just 1 or 2). Also, what is "every now and then"? Every five minutes, a few times per hour or once a day? Other than that: Have you finished everything you wanted to do? If so, I would merge the PR. And thanks a lot for your efforts!

DE-cr commented 2 years ago

Yes, I've finished what I wanted to do.

As for the package loss rate: I've now run a test with my standard six parameters (8830,8770,8700,8005,8003,8001) and a log intervall of 5 seconds. After 1000 log events, the result is as follows (number of parameters/packages received for each log event, with 6 (=full set) replaced by a period for better readability):

...........................................5...... .................................................. .................................................. .................................................. .................................................. .................................................. .................5................................ .................................1................ .................................................. .................................................. .................................................. .................................................. .................................................. .....5...................5..............5......... .................................................. .................................................. .................................................. .................................................. .........................................5........ ..................................................

This means that in this test, 6*1+1*5==11 out of 1000*6 packages got lost, i.e. less than 0,2 percent.

FYI: My BSB-LAN unit's wifi signal is reliable, but definitely not the strongest, as the unit is placed within the boiler's housing and on a different floor than the router. Also, its signal strength is not 100% stable. This may or may not explain the package loss rate here.

DE-cr commented 2 years ago

...and in another test run (same settings): .................................................. .................................................. .................................................. .................................................. .................................................. ..............................5................... .................................................. .................................................. ...................................3.............. .................................................. .................................................. .................................................. .................................................. .................................................. ...........................5...................... .................................................. .................................................. .................................................. .................................................. .................................................. I.e. 5/6000 packages lost <= 0.1 %

Here's my test script, in case you want to run your own test on your system(s):

#!/usr/bin/perl -w \

use strict;
use warnings;
use IO::Socket::INET;

++$|;
my $socket = IO::Socket::INET->new( LocalPort=>6502, Proto=>'udp' );
my $prev = time;
my $now;
my $n = 0;
my $nn = 0;

while ( $socket->recv( $_, 99 ) ) {
  $now = time;
  if ( $now - $prev > 2 ) {
    print $n if $nn;
    print "\n" unless !$nn || $nn % 50;
    exit if ++$nn > 1000;
    $n = 0;
    $prev = $now;
  }
  ++$n;
}
fredlcore commented 2 years ago

Great, thanks a lot for this intensive testing! This looks good (at least acceptable) to me, so I'll merge this now :)...