Skip to content
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 retries if the stratum hostname is not found #29

Closed
wants to merge 0 commits into from

Conversation

SatsForFreedom
Copy link
Contributor

After restoring the Wi-Fi connection due to a power outage, the router may still try to connect to the internet. Therefore, it will fail to solve the IP address, and then the *ipaddr returns a NULL pointer. This will crash the firmware when it reads from a NULL pointer.

@benjamin-wilson
Copy link
Collaborator

Why was bDNSFound changed from a bool to an int?

@SatsForFreedom
Copy link
Contributor Author

Because I need 3 states: waiting, success and failure.

Copy link
Collaborator

@johnny9 johnny9 left a comment

Choose a reason for hiding this comment

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

Don't create merge commits. When updating your PR rebase instead.

@@ -18,7 +18,7 @@

static const char *TAG = "stratum_task";
static ip_addr_t ip_Addr;
static bool bDNSFound = false;
static int8_t bDNSFound = 0;
Copy link
Collaborator

@johnny9 johnny9 Sep 10, 2023

Choose a reason for hiding this comment

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

this variable is no longer a bool, give it a clearer name and use an enum for the different states or have a second bool for tracking dns progress

@@ -704,6 +704,11 @@ task_result *BM1366_proccess_work(void *pvParameters)

GlobalState *GLOBAL_STATE = (GlobalState *)pvParameters;

if (GLOBAL_STATE->valid_jobs[rx_job_id] == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

create a new PR for this

Copy link
Collaborator

Choose a reason for hiding this comment

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

I merged this piece already. Can you update the PR without it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants