8000 Chart data source selection by terratec · Pull Request #955 · bitaxeorg/ESP-Miner · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Chart data source selection #955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Chart data source selection #955

wants to merge 10 commits into from

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 •
edited
Loading

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
Contributor
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 •
edited
Loading

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
Contributor
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 requested changes May 30, 2025
View reviewed changes
@mutatrum mutatrum added the enhancement New feature or request label May 30, 2025
@duckAxe
Copy link
Contributor
duckAxe commented May 30, 2025 •
edited
Loading

@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
Contributor
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
Contributor
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
Contributor
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
KillerInk
KillerInk reviewed Jun 10, 2025
View reviewed changes
KillerInk
KillerInk reviewed Jun 10, 2025
View reviewed changes
@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
Contributor
duckAxe commented Jun 12, 2025

@terratec 👍

@terratec terratec force-pushed the chart_data_source_selection branch from f72c633 to 77cb3f6 Compare June 12, 2025 19:56
@terratec terratec force-pushed the chart_data_source_selection branch from 45d92eb to 8caceaf Compare June 18, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KillerInk KillerInk KillerInk left review comments

@mutatrum mutatrum mutatrum requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@terratec @mutatrum @skot @duckAxe @KillerInk
0