Proposal to improve /DG implementation

[DE-cr]

Changed /DG data plot implementation to use C3.js on top of D3.js (both under MIT license):

Pros:

• better legibility for value numbers with plot lines close to each other (mouseover on plot)
• user can interactively highlight plot lines for improved overview (mouseover on legend entries)
• user can interactively disable plot lines for improved overview and vertical scaling (click on legend entries)
• added zoom (mousewheel/pinch on plot) and pan capability (drag zoomed-in plot)
• saves over 6 KB in compiled BSB-LAN sketch (-6708 Bytes on my ESP32)

Cons:

• when logging multiple parameters, plot lines will often have small gaps (reason: param sets not always logged within the same hh:mm; I have not figured out how to get around this display annoyance)
• client browser has to also load C3.js (currently 204 KB) in /DG call

Remaining issues (already existing in current D3-only implementation):

• favicon.svg turned invisible on /DG page ('path{fill:none}' needed for plot)
• no vertical and limited horizontal plot auto-resize to fit browser window

Tests:

• tested with datalog.txt sizes up to 1.2 MB / 18641 lines
• tested with logging of a single parameter and logging of six parameters
• tested using Chrome@Android+Ubuntu, Edge@Windows11, Firefox@Ubuntu, Safari@MacOS, Silk@Android
• tested using display rotation on Android
• tested using window sizes from ridiculously small ;) to UHD

Bonus:

• added scripts/BSB-LAN_datalog-viewer.html for analogous display of downloaded /D (datalog.txt), to be used on client system, without server interaction (to try out the new /DG implementation, open this file in your web browser, then load any datalog.txt file you have saved via /D off your BSB-LAN system)

[DE-cr]

Android Cell Phone Screenshot fyi

[DE-cr]

With some trickery, I've now fixed the line gap issue mentioned above - at the expense of an additional 16 Bytes in the BSB-LAN sketch binary. :)

What is the best way to offer the improved version for inclusion in BSB-LAN? Closing this pull request and creating a new one?

[fredlcore]

Thanks! This looks good so I think I'll merge it, but what do you mean with "What is the best way to offer the improved version for inclusion in BSB-LAN? Closing this pull request and creating a new one?" I would just merge this PR, so why the need for a new one?

[DE-cr]

When I originally created this pull request, the line gap problem still existed. I've since fixed it in my BSB_LAN fork. Will that fix also be included when you merge, or will I have to create a new/additional pull request?

Btw, I've also figured out how to resolve the invisible favicon on the /DG page. That fix will cost an additional 4 bytes for the sketch binary. I'll include it in my fork now...

[fredlcore]

Well, if you push it here, it will be included. Looking at "Files changed" above, that seems to be the case, but please double-check. Once you're done let me know an I'll merge.

[DE-cr]

Commits=8 looks good, i.e. my recent fixes should be included.

[DE-cr]

Commits ahead is now at 10: I've replaced some code that has become obsolete with my line gap fix, for a sketch binary size reduction of 29 Bytes.

[dukess]

Hi! I want to insert two words: i propose to save D3 library too and add preprocessor directives.

#if defined (D3_LIBRARY) 
...
#elif defined (C3_LIBRARY) 
...
#endif

[DE-cr]

While I personally don't see any advantages in the D3-only implementation, I don't think giving the users a choice between it and my D3+C3 proposal would hurt. :)

[DE-cr]

Btw, should you decide to keep the D3-only implementation (as an alternative to D3+C3 or as the only option): The fix for the invisible favicon on the /DG page is easy: html_strings.h line 111: "path { \n" -> "div path { \n"

[DE-cr]

...and a considerable amount of bytes can be shaved off the sketch binary with the D3-only implementation by keeping whitespace and comments out of the string containing the html/javascript code.

For a start, running the following on the D3-only implementation section in html_strings.h saves 1682 bytes: perl -pe 's/^"(\s+)/$1"/; s!(\s*//\s.+)\n"!"$1!; s/^"(\n)?"//'

More can be saved by manually going through the implementation (or by using a more sophisticated automation).

The price for this sketch binary code saving is reduced legibility in the html received by the BSB-LAN client, but who reads that anyway?

[dukess]

You right but if we want to improve code it would be pain for our brains :)

[DE-cr]

No, it wouldn't. Just read the Code in html_strings.h, which can contain comments and indentation, as in my proposal here! (Or use a pretty printer for Javascript.)

[DE-cr]

In case you haven't tried it: Passing the D3-only implementation for /DG through the perl one-liner above creates the text attached here. In my personal opinion, readability is not affected by this 1682 bytes code reduction. In fact, I would consider an additional removal of most of the \n at/as the line endings in the string pieces an improvement in readability. However, it seems that a few of them are actually required to have the javascript code execute properly. I haven't bothered to look which ones those are. Also, there's a few multi-line comments which could be moved out of the html code into the surrounding C code of html_strings.h itself, ...

html_strings_D3_shortened.txt

[fredlcore]

Thanks for the suggestion regarding stripping whitespace etc. from the HTML output. I think the code output in your attachement is readable fine, so please push a properly working version of this into this PR.

[DE-cr]

I had already tested the *.txt posted above to work properly. :) I'll push a complete html_strings.h containing that change then.

Just to make sure that there's no misunderstanding: What you see in this suggestion is code INPUT (C code in html_strings.h), not code OUTPUT (html sent to the BSB-LAN client). The latter will be stripped off the indentation and comments found in html_strings.h

This is also what I did in my D3+C3 implementation originally discussed here, besides the following sketch binary reducing measures:

1. absolutely no unnecessary whitespace in the code OUTPUT (this does reduce readability of the code INPUT slightly)
2. ...including line endings/separation (imo this actually increases readability in the code INPUT by avoiding \n at the end of each line's string)
3. short variable names (affects readability, but with the shortness of my D3+C3 code and one explanatory comment added, I consider it acceptable)
4. using single quotes instead of double quotes in the html (this does NOT reduce the sketch binary size, but imo greatly improves readability of the the code INPUT, as double quotes would have to be escaped via backslash within C strings; javascript style parsers complain about this, but the code is executed just fine on all browsers I've tested (see above))

I've intentionally kept two unnecessary semicolons in my code - in places where I consider them supportive of readability/maintainability.

1-3 are also used in the used javascript libraries' "min" versions, btw - by their authors/distributors. This is what reduces transmission time for them, but makes them hard to read.

With the length and complexity of the original D3-only implementation for BSB-LAN's /DG feature, I suggest to refrain from using measures 1+3 on it.

[fredlcore]

Yes, code-readability in the output will be affected, that's clear.

[DE-cr]

Done:

• user can select in BSB_LAN_config.h between D3 and D3+C3 implementation for /DG
• D3-only shortened through whitespace/comments moved out of html strings
• D3-only: invisible favicon fixed
• an additional easy 100 bytes shaved off the BSB-LAN sketch binary by removing unnecessary trailing zeros and newlines in the other html strings
• tested on my system (ESP32)

[DE-cr]

BTW, the ESP32's OTA update functionality is VERY handy in trying out implementation changes. :)

[fredlcore]

Great, thanks a lot! Will merge now...

[fredlcore]

Hm, the CI jobs fail after merging, does anyone of you have an idea what the problem could be, @DE-cr or @dukess?

[fredlcore]

Ok, it could either be this: https://github.com/actions/checkout/issues/36 Or @DE-cr has removed his remote branch before the merge. In any case, I think it'll work nevertheless...

[DE-cr]

The only thing I can think of that I cannot rule out easily is that I might have removed one or the other \n too many in the non-/DG sections of html_strings.h. If that's the case, please accept my apologies for not having tested enough!

Feel free to use the attached file instead of the current html_strings.h, which does revert this one particular change I made. This should increase the sketch binary by 48 bytes again, but otherwise shouldn't affect anything.

html_strings.txt

(No, I have not removed anything concerning by BSB-LAN fork.)

[fredlcore]

No, it's not an error in the code, it's an error in the merging process. But since I just now pushed a change in the changelog and now the error is no longer coming up, it's probably just been a hiccup somewhere down the line.

[DE-cr]

/CI is config saving, right? Just fyi: a few weeks ago this had also stopped working on my BSB-LAN ESP32. A complete re-initialisation of the unit solved the Problem.

[DE-cr]

...and config saving still does work on my system - with the latest files from my pull request / fork. (Just tested with disabling logging in /C.)

[fredlcore]

If this is an ongoing issue, please open up a seperate issue/bugreport for that, otherwise it'll get lost here.

[DE-cr]

No, that was just a single incident with my system, probably caused by a temporary coding error in my BSBmonCR project's customisation of BSB-LAN that had caused a buffer overflow.

[DE-cr]

When should chapter 4 in the BSB-LAN user manual adapted? Now? And where? Here or in https://github.com/1coderookie/BSB-LPB-LAN?

[fredlcore]

No worries, @1coderookie will take care of it once he has time for it.

[dukess]

Sorry for misunderstanding: I meant to give users option to choose between C3 and D3, but not using both libraries at the same time.

[DE-cr]

C3 is built on top of D3, it does not work without it.

[dukess]

Oh, clear, thanks.