15
0
Fork
You've already forked operame
0

Added option for sparkline #7

Open
boekenwuurm wants to merge 1 commit from boekenwuurm/sparkline into main
pull from: boekenwuurm/sparkline
merge into: revspace:main
revspace:main
revspace:noduino
revspace:0x87
revspace:colorbar
revspace:aqc
revspace:abc
boekenwuurm commented 2021年01月28日 03:13:01 +01:00 (Migrated from github.com)
Copy link

Added option for a sparkline.

Added option for a sparkline.
Juerd (Migrated from github.com) requested changes 2021年01月28日 04:05:57 +01:00
Juerd (Migrated from github.com) left a comment
Copy link

Thanks for the PR; a sparkline would be a nice configurable option. I haven't tried it yet and won't accept the PR in its current form, but have provided some remarks.

Thanks for the PR; a sparkline would be a nice configurable option. I haven't tried it yet and won't accept the PR in its current form, but have provided some remarks.
@ -0,0 +1,150 @@
// Adaped version of ESParklines to sidestep random() issue in FixedPointsArduino library
Juerd (Migrated from github.com) commented 2021年01月28日 03:51:14 +01:00
Copy link

I strongly prefer not including forked libraries in the Operame repository, if it can be avoided at all. I've submitted a PR to fix the underlying library instead, at Pharap/FixedPointsArduino#71

I strongly prefer not including forked libraries in the Operame repository, if it can be avoided at all. I've submitted a PR to fix the underlying library instead, at Pharap/FixedPointsArduino#71
Juerd (Migrated from github.com) commented 2021年01月28日 03:52:21 +01:00
Copy link

Nitpick: the other variables are called _enabled. For consistency, _enabled would be preferred to _enable.

Nitpick: the other variables are called `_enabled`. For consistency, `_enabled` would be preferred to `_enable`.
Juerd (Migrated from github.com) commented 2021年01月28日 03:52:31 +01:00
Copy link

Spelling: length

Spelling: length
@ -68,6 +75,10 @@ void display_big(const String& text, int fg = TFT_WHITE, int bg = TFT_BLACK) {
sprite.setTextColor(fg, bg);
sprite.drawString(text, display.width()/2, display.height()/2);
Juerd (Migrated from github.com) commented 2021年01月28日 03:53:39 +01:00
Copy link

I'm guessing the 766 was supposed to be sparkline_bufferlength? That variable is declared and assigned but not used elsewhere.

I'm guessing the `766` was supposed to be `sparkline_bufferlength`? That variable is declared and assigned but not used elsewhere.
Juerd (Migrated from github.com) commented 2021年01月28日 03:56:22 +01:00
Copy link

I'm curious what the -5 is for.

I'm curious what the -5 is for.
Juerd (Migrated from github.com) commented 2021年01月28日 03:58:52 +01:00
Copy link

I'm wondering if having the sparkline in the demo is worth resetting the main history for. OTOH, this makes it consistent with accessing the portal (resetting the microcontroller is the only way out) and does make the demo include the sparkline.

I'm wondering if having the sparkline in the demo is worth resetting the main history for. OTOH, this makes it consistent with accessing the portal (resetting the microcontroller is the only way out) and does make the demo include the sparkline.
@ -388,6 +406,7 @@ void loop() {
every(5000) {
co2 = get_co2();
Serial.println(co2);
Juerd (Migrated from github.com) commented 2021年01月28日 04:00:39 +01:00
Copy link

Not translated.

Not translated.
@ -389,3 +407,4 @@
co2 = get_co2();
Serial.println(co2);
if (sparkline_enable ) display_sparkline.add(co2);
}
Juerd (Migrated from github.com) commented 2021年01月28日 04:03:02 +01:00
Copy link

Maybe the buffer length should not be user configurable, at least not without explaining what it does. If it is configurable, I think it would be slightly more user friendly to let them specify the buffer size in minutes.

Why 16383 and 512?

Maybe the buffer length should not be user configurable, at least not without explaining what it does. If it is configurable, I think it would be slightly more user friendly to let them specify the buffer size in minutes. Why 16383 and 512?
Pharap (Migrated from github.com) reviewed 2021年03月25日 04:41:11 +01:00
@ -0,0 +1,150 @@
// Adaped version of ESParklines to sidestep random() issue in FixedPointsArduino library
Pharap (Migrated from github.com) commented 2021年03月25日 04:41:11 +01:00
Copy link

See Pharap/FixedPointsArduino#74.

See Pharap/FixedPointsArduino#74.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin boekenwuurm/sparkline:boekenwuurm/sparkline
git switch boekenwuurm/sparkline

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff boekenwuurm/sparkline
git switch boekenwuurm/sparkline
git rebase main
git switch main
git merge --ff-only boekenwuurm/sparkline
git switch boekenwuurm/sparkline
git rebase main
git switch main
git merge --no-ff boekenwuurm/sparkline
git switch main
git merge --squash boekenwuurm/sparkline
git switch main
git merge --ff-only boekenwuurm/sparkline
git switch main
git merge boekenwuurm/sparkline
git push origin main
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
revspace/operame!7
Reference in a new issue
revspace/operame
No description provided.
Delete branch "boekenwuurm/sparkline"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?