Skip to content

Chart data source selection#955

Merged
WantClue merged 23 commits intobitaxeorg:masterfrom
terratec:chart_data_source_selection
Sep 19, 2025
Merged

Chart data source selection#955
WantClue merged 23 commits intobitaxeorg:masterfrom
terratec:chart_data_source_selection

Conversation

@terratec
Copy link
Contributor

All data sources can be selected on-the-fly for the chart (y or y2 axis).

@mutatrum
Copy link
Collaborator

Can you add some screenshots?

@terratec
Copy link
Contributor Author

Settings with source selection:
settings

@terratec
Copy link
Contributor Author

Fan speed and rpm (manual and PID):
fan

@terratec
Copy link
Contributor Author

Power, current and voltages:
power

voltage

Maybe the suggestedMax can be improved.

@terratec
Copy link
Contributor Author

heap+wifi

@mutatrum
Copy link
Collaborator

mutatrum commented May 27, 2025

Thanks for the pictures, makes it a lot more clear!

Settings with source selection: settings

On initial though, I think the axis selection should be on the dashboard, near the graph, not in settings. Not sure how that should be UI wise.

Secondly, someone on Discord is working in the same area, but has a different direction: https://discord.com/channels/1091348375301013615/1094385595981246535/1375742408045760595

@terratec terratec force-pushed the chart_data_source_selection branch from e7a714b to 894d10e Compare May 29, 2025 13:23
@skot
Copy link
Collaborator

skot commented May 29, 2025

This is awesome! But I agree the source selector should be in the chart page.

@duckaxe
Copy link
Collaborator

duckaxe commented May 30, 2025

This is awesome! But I agree the source selector should be in the chart page.

I experimented with the chart display and have some suggestions. We add the two data source dropdowns (the border color is adjusted accordingly) and remove the labels from the chart (or reposition them). In dropdowns we should add an additional (empty) option (--) so that the user has the option to display only one source/axis.

Screenshot 2025-05-30 at 10 06 01 Screenshot 2025-05-30 at 10 06 22 Screenshot 2025-05-30 at 10 06 39 Screenshot 2025-05-30 at 10 06 54 Screenshot 2025-05-30 at 10 08 39 Screenshot 2025-05-30 at 10 09 04 Screenshot 2025-05-30 at 10 12 15

@mutatrum
Copy link
Collaborator

mutatrum commented May 30, 2025

One top left and the other top right, above their respective axes? No need to have the labels then.

Mock-up:
image

And if you're really OCD, put the caret on the left for the right axis 😆

@duckaxe
Copy link
Collaborator

duckaxe commented May 30, 2025

Ah, I dont see, you uploaded a mockup ;)
I have scaled the dropdown a little, it doesn't have to be big on the dashboard.

localhost_4200_

@mutatrum mutatrum added the enhancement New feature or request label May 30, 2025
@duckaxe
Copy link
Collaborator

duckaxe commented May 30, 2025

@terratec I added my changes to the two dropdowns on the dashboard. It works fine. Just apply the attached patch file to your code. What we need is to reset the chart dataset for the changed axis. See my video at the end.

Screen.Recording.2025-05-30.at.19.46.13.mov

@terratec
Copy link
Contributor Author

This is fantastic, thank you!

Maybe we can call initializeChart to reset? Do we have to deinit/unsubscribe first?

@duckaxe
Copy link
Collaborator

duckaxe commented May 31, 2025

@terratec I tested your latest changes, it looks like the view is reloading, but the chart is still broken.

Screen.Recording.2025-05-31.at.18.20.54.mov

@terratec
Copy link
Contributor Author

@terratec I tested your latest changes, it looks like the view is reloading, but the chart is still broken.
Screen.Recording.2025-05-31.at.18.20.54.mov

This is only a limitation with the static test data in system.service.ts.

@duckaxe
Copy link
Collaborator

duckaxe commented May 31, 2025

@terratec I changed something and it looks much better without re-rendering the view. Untitled.patch

Untitled2.mov

@terratec
Copy link
Contributor Author

@terratec I changed something and it looks much better without re-rendering the view. Untitled.patch
Untitled2.mov

With this change the previous data is missing. I think we need PR #951 and then we can also move "load previous data" into a separate function.

@duckaxe
Copy link
Collaborator

duckaxe commented May 31, 2025

Ah, you right. Okay, then we'll wait for the merge.

@mutatrum mutatrum added this to the 2.9.0 milestone Jun 4, 2025
@terratec terratec force-pushed the chart_data_source_selection branch 2 times, most recently from ed5a040 to f72c633 Compare June 6, 2025 21:50
@terratec
Copy link
Contributor Author

@duckaxe With #996 a new subscription for this.info$ was added. It looks like the interval object now survives the optimized reload logic.
Can I fix it like this or is there a better way?
this.titleSubscription = this.info$.subscribe(info => { this.titleService.setTitle( [ ...
and in updateSystem():
... next: () => { this.titleSubscription.unsubscribe(); ...

@duckaxe
Copy link
Collaborator

duckaxe commented Jun 12, 2025

@terratec 👍

@terratec terratec force-pushed the chart_data_source_selection branch 2 times, most recently from 45d92eb to 8caceaf Compare June 18, 2025 19:08
Copy link
Collaborator

@mutatrum mutatrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, hope we can get this merged in the next version. Is there stull stuff to do as it's in draft?

@mutatrum mutatrum added this to the 2.11.0 milestone Sep 5, 2025
@terratec
Copy link
Contributor Author

terratec commented Sep 5, 2025

What about the hashrate calculation in the frontend html? Extra Issue?

@terratec terratec marked this pull request as ready for review September 5, 2025 22:30
@WantClue
Copy link
Collaborator

What about the hashrate calculation in the frontend html? Extra Issue?

yes this should be it's own issue please.
I'm making sure to review and test this PR so that we can merge this ASAP

@WantClue WantClue self-assigned this Sep 12, 2025
.pipe(this.loadingService.lockUIUntilComplete())
.subscribe({
next: () => {
this.titleSubscription?.unsubscribe();
Copy link
Collaborator

@duckaxe duckaxe Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need an unsubscribe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each source selection would add a subscription with additional data points.

@duckaxe
Copy link
Collaborator

duckaxe commented Sep 15, 2025

terratec#1

Another idea would be to synchronize the two dropdowns. When something is selected in one dropdown, remove that entry from the other dropdown. But nice to have.

@duckaxe
Copy link
Collaborator

duckaxe commented Sep 19, 2025

Tested on my Bitaxe. Looks 👍

@WantClue WantClue merged commit a0e8757 into bitaxeorg:master Sep 19, 2025
3 checks passed
@terratec terratec deleted the chart_data_source_selection branch September 22, 2025 20:44
@kakulukia
Copy link
Contributor

Dont really know if this is the source of the problem, but as its realated i just wanted to report it here.

I installed the current git version and really like the new dashboard and also that it kinda took the freedom to auto activated data logging to not start without data to display.

But now every time i visit the settings page data logging is set to "Every 10 minutes, 28 seconds for 5 days, 5 hours, 43 minutes, 12 seconds" not respecting the previous setting.

Was that introduced with the merge of this PR or where does that actually belong to?

Another side note: The dropdown entries for switching the data source look rather random - whats the logic behind that?

@terratec
Copy link
Contributor Author

Dont really know if this is the source of the problem, but as its realated i just wanted to report it here.

I installed the current git version and really like the new dashboard and also that it kinda took the freedom to auto activated data logging to not start without data to display.

But now every time i visit the settings page data logging is set to "Every 10 minutes, 28 seconds for 5 days, 5 hours, 43 minutes, 12 seconds" not respecting the previous setting.

Was that introduced with the merge of this PR or where does that actually belong to?

Another side note: The dropdown entries for switching the data source look rather random - whats the logic behind that?

The default value for data logging should be 0 (disabled). This seems to be an user-defined value or maybe an old value from an older test version.
First I would try setting the value to 0, clicking save and restart.

The dropdown boxes contain all available data sources from the data logging task.

@kakulukia
Copy link
Contributor

Saving and restarting did not change the behaviour. Im on 1873664-dirty as i was testing the autotune feature.

Saving the 0 is respected since i dont see content in the chart right after starting, but as soon as i visit the systems settings the displayed valued is not initialized with the current value of the setting.

And my question about the dropdown content was more about: Why is it not sorted alphabetically? Its at least seems random to me if there was a reason to sort it that way: id like to know.

@terratec
Copy link
Contributor Author

Saving and restarting did not change the behaviour. Im on 1873664-dirty as i was testing the autotune feature.

Saving the 0 is respected since i dont see content in the chart right after starting, but as soon as i visit the systems settings the displayed valued is not initialized with the current value of the setting.

And my question about the dropdown content was more about: Why is it not sorted alphabetically? Its at least seems random to me if there was a reason to sort it that way: id like to know.

Maybe it's a side effect of the merge. Have you also tested an image from the master branch?
Probably info.statsFrequency is not yet fully fixed @KillerInk
257e246

The chart is always empty after booting because RAM is used for data logging.

Ah yes the order is:

  • system default values
  • temperature related
  • voltage related
  • power related
  • fan related
  • system related

@kakulukia
Copy link
Contributor

What about also displaying the grouping that way: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/optgroup

No, I have not yet tested the master branch. I will have another look after 2.12 is done as it's not that serious.

@terratec
Copy link
Contributor Author

Something like this: https://v17.primeng.org/dropdown#group
Maybe create an issue?

@duckaxe
Copy link
Collaborator

duckaxe commented Oct 18, 2025

Grouping would require quite a bit of technical restructuring (eChartLabel must become an array, the related code must be refactored). Other simple ways would be to sort the options alphabetically or use a frequency-based order (values that are rarely used are placed at the bottom).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants