ESP32 issues

[joaquinbvw]

Hi! I'm a little bit new to Duino Coin, I have to say I am very interested on this project and hope it gets bigger with time. Maybe we can see DUCO on Binance soon!

So the first thing I wanted to do was to try to mine with the ESP32 and found some issues with the code that I think could be improved. Maybe there is a reason to why it was implemented the way it is right now that's why I'm writing it first here. I will list some of them with some suggestions on what can be done:

• The tasks:
  From what I know about RTOS is that you don't usually want to kill the tasks unless is very very necessary and it is done rarely, in lines 188 and 108 the tasks kill themselves. Since right now the server is requesting a reconnection frequently the tasks will be killing themselves a lot of times. What I think should be done is to wrap everything inside the task on an infinite loop with a "for(;;)", I have test this without any problems whatsoever.

• The variables, arrays and pointers:
  When we program on microcontrollers we have to think about the limited memory (even for the ESP32). So declaring the same variable or array multiple times could be asking for problems in the future. Some people could say that maybe the compiler can resolve this but trusting on the compiler to do something that you aren't really sure if he is gonna do is not a good decision. Even worse if you are reserving memory every time for an array or pointer, this can become on a memory problem, if you don't free the allocation the memory will become full and even if you use "free" to deallocate the memory it will become fragmented and would become a major problem on long runs. What should be done is to declare all variables once, they can be declared globally or locally inside the task, but outside the infinite loop. The memory allocation too, but there is something else that can be improved, in lines 84 and 162 instead of using a constant pointer (which is not really constant since it could change on every hash calculation, I'm not really sure what happens here or how the Arduino or the compiler handles this but it seems to workout somehow) you can use ".c_str()" directly so it would become something like this:
  "esp_sha(SHA1, (const unsigned char*)hash11.c_str(), payloadLenght1, shaResult1);"
  I have tested it without any problems and with this you are not messing with pointers directly anymore, the Arduino does it on the background for you.
  https://arduino.stackexchange.com/questions/682/is-using-malloc-and-free-a-really-bad-idea-on-arduino

• The esp_sha function:
  I can understand why you just use the esp_sha function in one of the tasks, the tasks could call this at the same time. I think the RTOS or the processors can handle this without problems. Right now I am using the esp_sha function on both tasks without problems, I think it is even faster than before. This is a suggestion that should be tested more since I don't have a strong argument on why it works like that.

• The WiFiClient buffer and reconnection:
  I'm not really sure why but the server seems to close the connections to the ESP32 (maybe this is not only related to the ESP32 but I didn't try another miner) very frequently and this is causing other problems on the code too. The first thing I realized about the client handling is that the code is not waiting enough time to receive the response from the server, it sends a request and right after this it tries to read the buffer. I'm not sure if this is handled by the library or on hardware or if it just isn't handled at all, I just think that is a good practice to wait for a response from the server, since you are the one that are starting the transaction. So what I'm doing is to put this portion of code after lines 59 and 132:

    while(!client1.available()){
      if (!client1.connected())
        break;
      delay(10);
    }
    yield();
    if (!client1.connected())
      break;
    delay(100);
    yield();
    int buff1size = client1.available();
    Serial.print("CORE1 Buffer size is ");
    Serial.println(buff1size);
    if (buff1size<=10) {
      Serial.println("CORE1 Buffer size is too small. Requesting another job.");
      continue;
    }

So there is something more on this code, besides waiting for the response on the while loop it is breaking the upper while if the connection is lost so it can connect again. In addition to this I am adding some "yield()" functions to manage the WiFi connection after potentially long loops or waiting times. And at the end I am validating the buffer size to be more than 10 bytes, if it isn't it returns to the beginning of the upper while.
There is other time that the client posts something to the server and it is when it finds a result, in this case I do it differently, here is the code I did that goes before the lines 92 and 172:

        if (!client1.connected()) {
          Serial.println("CORE1 Lost connection. Trying to reconnect");
          if (!client1.connect(host, port)) {
            Serial.println("CORE1 connection failed");
            break;
          }
          Serial.println("CORE1 Reconnection successful.");
        }

And here is the code after lines 92 and 172:

        Serial.println("CORE1 Posting result and waiting for feedback.");
        while(!client1.available()){
          if (!client1.connected()) {
            Serial.println("CORE1 Lost connection. Didn't receive feedback.");
            break;
          }
          delay(10);
          yield();
        }
        delay(100);
        yield();

The code that goes before is for trying to reconnect if it lost the connection, I do this because if it finds a solution it should post it so it should try at least one reconnection before going out of the loop. If the reconnection is not successful then it leaves the loop. For the next part of the code it waits for a response from the server like before. As I'm writing this there could be a problem with this approach, since the server sends the version of the miner immediately after connecting it could be mistaken for a feedback response. Maybe I can work this around later, but it is better like this even if it is mistaken.

Finally I have been playing with the timeout value for the clients, right now it is 1 second and it is working fine besides the reconnections from the server. Another thing I did was putting more yield() functions here and there, where there is a potentially long waiting time.

Oh another suggestion that can improve even more the code is not using the String class. I know it is convenient sometimes but is very messy and I think it uses more memory than it should use, if we use just char arrays or pointers I think it could improve the code and we can even be closer to porting the code to another platform different than the Arduino.

One last suggestion is to add a WiFi reconnection task, I have been dealing with a poor WiFi connection at home and it's been a pain.

I'm sorry for the long post but I wanted to to detail everything I could about the things I have been improving. Please let me know if something I'm doing is wrong or if I need to change something else. Thank you!

[revoxhere]

Really nicely explained! We will try to make use of these tips, @JoyBed should also take a look

[joaquinbvw]

Great! I hope it helps.

By the way there is a couple of things that I forgot, one is the flushing of the client, this should be done before sending any request to the server and after the reading of the buffer. Doing it like this would help preventing the mixing of messages (for example the server version could be merged with the actual job causing a bad share).

Yesterday I was making some tests and find out that there were a lot of bad shares, I was thinking that maybe it was the esp_sha function but playing with the yield and some delays I found out that somehow the esp_sha need some wait time before and after execution, I don't know why it behaves like that but when I put some delays before and after suddenly there were not one bad share. And it didn't affect the hashrate, the delays I use are 10 microseconds before and after each esp_sha call.

Finally another suggestion is to stop using the delay functions, I think the RTOS handles that well on the background, but this is just for the ESP32, it's a little bit messy to use the delay function on a task. This is somewhat of an opinion more than a fact actually haha.

Will be looking forward to any change in the future.

[joaquinbvw]

In case someone want the code I'm running here it is. It seems to be working great without any bad shares or close to zero for long runs. I have added WiFiMulti reconnection too. Btw, I have changed the baudrate too, it's 500K, the ESP32 can handle that speed nicely.

DuinoCoinESP32.txt

[xscode]

Tried your new code and it dies after several minutes every time, not sure why as I'm not saving the serial output.

[joaquinbvw]

I will post it without WiFiMulti, I think is making the code crash.

[joaquinbvw]

Try this one. I didn't remove the WiFiMulti (it is quite useful) I have change some lines and I think it works but let me know if you have some problems.

By the way, the server will always close the connection from time to time (could be several minutes or some hours), I think this behavior is expected and after that it should connect immediately. What it should not be expected is to disconnect from WiFi or don't connect to WiFi at all.

DuinoCoinESP32.txt

[vicko03]

is not working

[vicko03]

@joaquinbvw ....is not working.

[joaquinbvw]

@joaquinbvw ....is not working.

Ok, what do you see on the serial monitor? Is it connecting to WiFi or not connected at all? I think that code I posted had some issues with some boards and also I don't like it very much that's why I didn't put a request. Nevertheless I have requested a code change now, maybe you can try that code instead. Here it is:

https://github.com/revoxhere/duino-coin/blob/9fbee1b3612976c3dd8e3005d82ca2203874c3c6/ESP32_Code/ESP32_Code.ino

[joaquinbvw]

I believe I have implemented all of these optimizations already on the latest version. Will close it now.