Proposal to improve /DG implementation #459

DE-cr · 2022-06-16T08:59:50Z

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)

improved /DG implementation

DE-cr · 2022-06-16T09:11:47Z

Android Cell Phone Screenshot fyi

got rid of gaps in plot lines (which were due to hh:mm differing within param sets)

removed tab in last edit

removed tab in source code

DE-cr · 2022-06-16T16:41:52Z

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 · 2022-06-16T19:18:23Z

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 · 2022-06-16T19:52:09Z

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...

fixed invisible favicon at the top of the /DG page

synchronized with ../html_strings.h (although not really required here)

fredlcore · 2022-06-16T19:55:16Z

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 · 2022-06-16T20:01:19Z

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

removed obsolete code (saves 29 Bytes in sketch binary)

code synchronized with ../html_strings.h

DE-cr · 2022-06-17T07:49:46Z

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.

cosmetics (src=https synced, decided to use the shorter http)

synced with ../html_strings.h

replaced ";}" with "}" in single-line "{...}" - doesn't affect readability/maintainability, but saves a few bytes in the sketch binary

sync'ed with ../html_strings.h

dukess · 2022-06-17T16:43:15Z

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

Oops, lost one important ';' some commits ago.

sync'd with ../html_strings.h (although not important here)

DE-cr · 2022-06-17T17:08:47Z

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. :)

Using https instead of http for remote javascript libraries, to avoid redirect and allow browser caching

sync'd with ../html_strings.h

DE-cr · 2022-06-18T06:40:35Z

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 · 2022-06-18T08:43:30Z

...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 · 2022-06-18T17:53:50Z

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

DE-cr · 2022-06-18T19:31:23Z

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 · 2022-06-18T20:13:44Z

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 · 2022-06-18T21:26:59Z

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 · 2022-06-19T06:21:45Z

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:

absolutely no unnecessary whitespace in the code OUTPUT (this does reduce readability of the code INPUT slightly)
...including line endings/separation (imo this actually increases readability in the code INPUT by avoiding \n at the end of each line's string)
short variable names (affects readability, but with the shortness of my D3+C3 code and one explanatory comment added, I consider it acceptable)
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 · 2022-06-19T07:14:47Z

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

re-introduced the original D3-only implementation for log file plotting (/DB) as an option (used unless #ifdef USE_ADVANCED_PLOT_LOG_FILE) ... and reduced its BSB-LAN binary code footprint by 2256 bytes through moving whitespace and comments out of the html strings ... and also fixed the invisible favicon in the D3-only version more sketch binary code savings achieved by: - removing trailing zeroes (after the decimal point) in the svg code for the favicon => -52 bytes - removing obviously unnecessary \n characters in the older html strings => -48 bytes

introduced "#define USE_ADVANCED_PLOT_LOG_FILE", which can be disabled to keep the older D3-only implementation for the /DG functionality

DE-cr · 2022-06-19T09:18:46Z

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 · 2022-06-19T09:23:16Z

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

fredlcore · 2022-06-19T09:27:48Z

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

fredlcore · 2022-06-19T09:40:24Z

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

fredlcore · 2022-06-19T09:46:33Z

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

DE-cr · 2022-06-19T10:10:46Z

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 · 2022-06-19T10:17:28Z

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 · 2022-06-19T10:20:19Z

/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 · 2022-06-19T10:24:02Z

...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 · 2022-06-19T10:26:18Z

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

DE-cr · 2022-06-19T10:33:22Z

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 · 2022-06-19T14:50:35Z

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 · 2022-06-19T14:54:26Z

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

dukess · 2022-06-19T16:57:16Z

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 · 2022-06-19T18:47:42Z

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

dukess · 2022-06-19T19:27:23Z

Oh, clear, thanks.

DE-cr added 2 commits June 16, 2022 10:45

Add files via upload

9d83a09

Update html_strings.h

968fad8

improved /DG implementation

DE-cr added 4 commits June 16, 2022 16:00

Update html_strings.h

030049e

got rid of gaps in plot lines (which were due to hh:mm differing within param sets)

Update BSB-LAN_datalog-viewer.html

50f391b

got rid of gaps in plot lines (which were due to hh:mm differing within param sets)

Update html_strings.h

ca9ce83

removed tab in last edit

Update BSB-LAN_datalog-viewer.html

825dbbb

removed tab in source code

DE-cr added 2 commits June 16, 2022 21:53

Update html_strings.h

9cee3db

fixed invisible favicon at the top of the /DG page

Update BSB-LAN_datalog-viewer.html

9d425a0

synchronized with ../html_strings.h (although not really required here)

DE-cr added 2 commits June 17, 2022 09:46

Update html_strings.h

b1780e3

removed obsolete code (saves 29 Bytes in sketch binary)

Update BSB-LAN_datalog-viewer.html

fe0f9e6

code synchronized with ../html_strings.h

DE-cr added 4 commits June 17, 2022 10:02

Update html_strings.h

ba8f6f2

cosmetics (src=https synced, decided to use the shorter http)

Update BSB-LAN_datalog-viewer.html

bb1d581

synced with ../html_strings.h

Update html_strings.h

612b4c2

replaced ";}" with "}" in single-line "{...}" - doesn't affect readability/maintainability, but saves a few bytes in the sketch binary

Update BSB-LAN_datalog-viewer.html

3ba0124

sync'ed with ../html_strings.h

DE-cr added 2 commits June 17, 2022 19:03

Update html_strings.h

fb78aed

Oops, lost one important ';' some commits ago.

Update BSB-LAN_datalog-viewer.html

95a9b5b

sync'd with ../html_strings.h (although not important here)

DE-cr added 2 commits June 18, 2022 08:30

Update html_strings.h

f1c1da3

Using https instead of http for remote javascript libraries, to avoid redirect and allow browser caching

Update BSB-LAN_datalog-viewer.html

9ca389c

sync'd with ../html_strings.h

DE-cr added 2 commits June 19, 2022 11:11

Update BSB_LAN_config.h.default

537bbe7

introduced "#define USE_ADVANCED_PLOT_LOG_FILE", which can be disabled to keep the older D3-only implementation for the /DG functionality

fredlcore closed this Jun 19, 2022

fredlcore reopened this Jun 19, 2022

fredlcore merged commit a1633d8 into fredlcore:master Jun 19, 2022

DE-cr mentioned this pull request Feb 21, 2023

[FEATURE REQUEST] Modifying the graph to allow the long-term data logging #539

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal to improve /DG implementation #459

Proposal to improve /DG implementation #459

DE-cr commented Jun 16, 2022

DE-cr commented Jun 16, 2022

DE-cr commented Jun 16, 2022

fredlcore commented Jun 16, 2022

DE-cr commented Jun 16, 2022

fredlcore commented Jun 16, 2022

DE-cr commented Jun 16, 2022

DE-cr commented Jun 17, 2022

dukess commented Jun 17, 2022

DE-cr commented Jun 17, 2022 •

edited

Loading

DE-cr commented Jun 18, 2022 •

edited

Loading

DE-cr commented Jun 18, 2022

dukess commented Jun 18, 2022

DE-cr commented Jun 18, 2022

DE-cr commented Jun 18, 2022

fredlcore commented Jun 18, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

fredlcore commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

dukess commented Jun 19, 2022

DE-cr commented Jun 19, 2022

dukess commented Jun 19, 2022

Proposal to improve /DG implementation #459

Proposal to improve /DG implementation #459

Conversation

DE-cr commented Jun 16, 2022

DE-cr commented Jun 16, 2022

DE-cr commented Jun 16, 2022

fredlcore commented Jun 16, 2022

DE-cr commented Jun 16, 2022

fredlcore commented Jun 16, 2022

DE-cr commented Jun 16, 2022

DE-cr commented Jun 17, 2022

dukess commented Jun 17, 2022

DE-cr commented Jun 17, 2022 • edited Loading

DE-cr commented Jun 18, 2022 • edited Loading

DE-cr commented Jun 18, 2022

dukess commented Jun 18, 2022

DE-cr commented Jun 18, 2022

DE-cr commented Jun 18, 2022

fredlcore commented Jun 18, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

fredlcore commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

DE-cr commented Jun 19, 2022

DE-cr commented Jun 19, 2022

fredlcore commented Jun 19, 2022

dukess commented Jun 19, 2022

DE-cr commented Jun 19, 2022

dukess commented Jun 19, 2022

DE-cr commented Jun 17, 2022 •

edited

Loading

DE-cr commented Jun 18, 2022 •

edited

Loading