-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
Add ASICs High temp to power management task #60
base: develop
Are you sure you want to change the base?
Conversation
Just to get the lower temps
…Miner-NerdQAxePlus into asic-high-temp-pm
POWER_MANAGEMENT_MODULE.setAsicHighTemp(ftemp); | ||
asic_temp = ftemp; | ||
} | ||
if ( asic_count == asic_nr ){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things I found:
-
First, the only ASICs supporting to read the chip temp is the 1368. The 1366 and 1370 won't ever return anything. This would lead to no overtemperature protection at all because the
m_asic_high_temp
always will be zero. Actually it would be random because the field is not initialized. -
Second, the code shouldn't assume there are returning temp responses in a defined order, it also shouldn't assume all asics repond with temperatures. The temp requesting is weird and I had fun with it getting it to work 🫣
ESP_LOGI(TAG, "asic high temp: %.3f", m_asic_high_temp); | ||
|
||
// to show the high temp in dashboard | ||
if (m_asic_high_temp > m_chipTempAvg) m_chipTempAvg = m_asic_high_temp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And third, the semantics is a little bit off, because the variable is called Avg
but it will get a max value. But it was off before I have to admit, because the average of a single temp sensor ... 🤔 🙈
@@ -17,6 +17,8 @@ void ASIC_result_task(void *pvParameters) | |||
Asic* asics = board->getAsics(); | |||
|
|||
char *user = nvs_config_get_string(NVS_CONFIG_STRATUM_USER, STRATUM_USER); | |||
float asic_temp = 0.0f; | |||
int asic_count = board->getAsicCount() - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the getAsicCount
call returns the number of asics the board should have but it doesn't take into account that less might be found. It's another reason why the if
might fail.
@@ -17,6 +17,7 @@ class PowerManagementTask { | |||
void task(); | |||
|
|||
public: | |||
float m_asic_high_temp; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be initialized with = 0.0f
in the constructor.
Take the high temperature into the power management task, to deal with differences in temperature readings.
I have a "lazy" ASIC (#0), it doesn't return any nonce, and its temperature is always lower than the others. So the temperature sensor returns almost 10 degrees lower than the other ASIC IC.