Add mining enable/disable API and UI #927
Conversation
WantClue
added
enhancement
|
Would it be a good idea to put the stop/start button on the dashboard? I think, no. This button isn't something anyone needs every day. I would put the button in the settings. After all, it's just a setting to turn the miner on or off, so it's not important to put it on the dashboard. |
|
@GIGIG4 Can you resolve the tiny conflict? And I agree with duckAxe, we need to think a bit on where in the UI it should sit. A big button on the dashboard doesn't feel right. It's a bit in the same category as the 'Restart' button, maybe add a toggle below that one in the sidebar? |
main/CMakeLists.txt
Outdated
| "./power/INA260.c" | ||
| "./power/power.c" | ||
| "./power/vcore.c" | ||
| "mining_controller.c" # Added mining controller |
There was a problem hiding this comment.
No need to add a comment here
main/global_state.h
Outdated
| bool ASIC_initalized; | ||
| bool psram_is_available; | ||
|
|
||
| // New fields for mining state and task handles |
There was a problem hiding this comment.
Commends should be timeless, remove the New.
| <p-button (click)="restart()" id="restart" label="Restart" severity="primary"></p-button> | ||
| </li> | ||
|
|
||
| </ul> |
There was a problem hiding this comment.
Try to avoid white-space edits in files you're otherwise not editing in this PR. Try to keep the number of changed files as low as possible.
|
|
||
| httpd_uri_t mining_start_options_uri = { | ||
| .uri = "/api/mining/start", | ||
| .method = HTTP_OPTIONS, |
There was a problem hiding this comment.
I'm not that familiar with the AxeOS api, so maybe a silly question, why does this need both an OPTIONS as well as a PATCH handler? Why is this not just a POST?
| VCORE_set_voltage((double) core_voltage / 1000.0, GLOBAL_STATE); | ||
| last_core_voltage = core_voltage; | ||
| } | ||
| // Read the state of plug sense pin |
There was a problem hiding this comment.
Whitespace here as well.
|
|
||
| static const char * TAG = "mining_controller"; | ||
|
|
||
| esp_err_t start_mining(GlobalState * global_state) |
There was a problem hiding this comment.
A general comment, it looks like this class has too much responsibility. Ideally, it should only toggle the flag, and start tasks, and the tasks themselves should handle all responsibilities. F.e. retrieving frequency and logging about it, that should be done inside POWER_MANAGEMENT_task.
Secondary remark: it looks like some stuff is duplicated from main.c. Ideally, the cold boot should use the exact same startup sequence and code as a restart.
| } else if (stratum_api_v1_message.method == CLIENT_RECONNECT) { | ||
| ESP_LOGE(TAG, "Pool requested client reconnect..."); | ||
| stratum_close_connection(GLOBAL_STATE); | ||
| if (!GLOBAL_STATE->mining_enabled) { |
There was a problem hiding this comment.
Why is this check and the stop block so many times in stratum_task.c? Is it possible to have one stop block per task? I see it logs a different stop reason, but in all cases an error is already logged, so having a specific stop reason doesn't add a lot.
timrosenbach
force-pushed
the
mining-controller
branch
from
fa3c4d9 to
2d913f8
Compare
mutatrum
changed the title
|
@GIGIG4 please resolve the conflict so we can proceed with reviewing it. |
WantClue
added
accepted
4 participants
This PR adds the option to disable and enable the mining without powering of the whole ESP.
Add two endpoints
/api/mining/stopand/api/mining/startas well as a card to the home.component.ts:When mining is resumed, it hovers at roughly 800 GH/s for a minute or two before going up to normal levels again.
Tested on my 601 for a week with multiple starts and stops.