v317 bugs and some suggestions

[Marco Gulino]

Hi @brownrb,
I recently tried to update some of my builds to 317, as some of them were using a really old version of the driver.

I noticed a few issues:
in eota_server.cpp, there's this line:

ElegantOTA.setAuth(ename, epwd);

which I had to comment out, as there's no such method in ElegantOTA 2.2.7. Authentication is done by the webserver itself, not eota, as far as I can see. There's also arguments in the begin() function of ElegantOTA for setting auth if needed.

In myfp2esp32_317.ino::eota_start():

  // Check if there is an instance of the class
  if (myeota_srvr) {
    return true;
  }

I'm not sure what was the intention of this check, but as it is written right now, it just returns "true" every time it's invoked, without actually starting the server.
Perhaps you wanted to check the opposite instead?

  // Check if there is an instance of the class
  if (!myeota_srvr) {
    return false;
  }

Lastly, in myfp2esp32_317.ino::setup(), there's this part for setting up the wifi, where first of all, if WiFiMulti is enable, it runs it (and reboots if it doesn't connect. Afterwards though, there's this part:

    // attempt to connect to user's wifi
    WiFi.begin(mySSID, myPASSWORD);
    WiFi.setSleep(false);
    delay(1000);
    for (int attempts = 0; WiFi.status() != WL_CONNECTED; attempts++) {
      delay(500);
      if (attempts > 9) {
        delay(500);
        break;
      }
    }

Basically it just tries to setup wifi AGAIN, using the defined SSID and PSK from the non-wifi-multi setup, without checking whether we're already connected. Which would result in the controller first connecting to wifi using wifimulti, then trying again with the default (unconfigured) SSID and PSK, and so disconnecting from wifi.
In here we should either check whether WiFi is already connected:

@@ -1497,6 +1522,8 @@
       boot_msg_println(T_DYNAMICIP);
     }
     // attempt to connect to user's wifi
+
+    if (WiFi.status() != WL_CONNECTED) {
      WiFi.begin(mySSID, myPASSWORD);
      WiFi.setSleep(false);
      delay(1000);
 @@ -1507,7 +1534,7 @@
          break;
        }
      }
 -
 +    }

or otherwise just skip this part entirely if WIFIMULTI is enabled.

There's also a couple of suggestions for improving the overall experience:
In setup(), just after load_vars() is called, it might be handy to do:

@@ -1423,6 +1447,7 @@
   load_vars();


+   WiFi.setHostname(devicename);
   //-------------------------------------------------
   // ACCESSPOINT START
   //-------------------------------------------------

This would register the microcontroller on the network with the configured device name (friendly and unique name) rather than the default myFP2ESP32. This way, if you have multiple controllers, you can connect to them using the hostname rather than IP address (example, esprit-100-focuser.local, c11-focuser.local, etc).

In setup(), there's this line:

  if (myboard != PRO2ESP32LOLINS2MINI) {
    while (!Serial) {
      ; 
    }
  }

Wouldn't it be better to check for

    #ifndef ARDUINO_LOLIN_S2_MINI
    while (!Serial) {
      ;
    }
    #endif
  }

instead? This way it relies on the actual board in use, rather than the configuration, used, which may or may not vary (in my case I'm using an S2 mini, but with a custom config, hence I can't really use PRO2ESP32LOLINS2MINI).

Lastly, could it be possible to use something like custom user includes for setting up your own config?
In one on my projects, I use

// config.h
// set up defaults
#if __has_include ("config_custom.h")
#include "config_custom.h"
#endif

This way it makes it a lot easier for users who have customised their code to upgrade to a newer version.

Hope this all helps :)
Marco

[Marco Gulino]

Something else I forgot:
On some of my boards I run the ESP32 C3 variant, which doesn't have the extra hardware timer.
This could be easily managed in tasktimer.h:

+#ifdef ARDUINO_LOLIN_C3_MINI
+# warning "ARDUINO_LOLIN_C3_MINI detected - switching to Timer0"
+  task_timer = timerBegin(0, 80, true);
+#else
   // using a define in place of 2, the task timer did not start
   task_timer = timerBegin(2, 80, true);
+#endif

[brownrb]

Hi Marco
Thanks for the posts, I appreciate the feedback.

ElegantOTA.setAuth(ename, epwd);
The reason why there is no Auth now is because the library editor removed those functions and now the cut-down version is free and if one wants auth and all the other stuff like custom UI then
one has to pay. often when these libraries or even the esp32 code is updated its bould to break multiple things.

if (myeota_srvr) {
return true;
}

myeota_srvr is a pointer to a class. So its testing to see of the pointer points to a valid instance of a class and so somewhere in the code this pointer will point to an instance of the class, until then, I dont think I need to spell that out.

In setup() line 1761
myeota_srvr = new EOTA_SERVER();

the pointer is assigned to a new instance of a class
Next there is a call to start the new instance via the class pointer myeota_srvr
ota_status = eota_start();

if (myboard != PRO2ESP32LOLINS2MINI) {
while (!Serial) {
;
}
}

replacing with ndefs becomes a little messy where there are multiple exceptions.
It easier in the code to assign the board_type directly in "myboard". As you will note throughout, there is a move away from the # conditional defines implemented at compile time. The move to using "myboard" just as an example is that most of the focuser code like temp probe, etc can be included into the "default configuration" - the idea behind that is so the user does not have to keep recompiling a new firmware every time some needs to be added or removed. hence you will note that things like pushbuttons and leds and temp probe etc do not require a reprogram of the controller.

In one of the much earlier videos I demonstrate that I can change the driver board to another type on the fly without having to recompile the code. From memory I changed a UNL2003 board with a L298N driver board.

In making the newer version on this site, some things fell by the wayside, but this auto-config and change on the fly is something that be implemented when I have the time.

Apologies, its 3:30am my time.

Please send me an example of the custom config.

For hostname etc this has been already implemented in the mySQM+ project - I attach pdf to show what that looks like, it jsut has not made its way into myFP2ESP32 yet. AS lot of the time I write code to so something and then reuse that code in another project.

Timers - the project was based on the esp32 dev 30p module. Yes, I know there are other variants out there, with less timers or only 1 cpu. But I cant support all the variants and just dont have the $ now to purchase variants etc.

But I am open to adding things. I think Eric did a lot on the S2 as well and I thiink thats now in the PDF and in the firmware code. So tweaking that is very possible. The big issue I have at present is the time it takes to release a new release. i am now talking months not weeks.

i will be back in touch a little later and I look through your comments and code suggestions. Once again thanks for those and I will give them consideration.

regards
Robert

[Marco Gulino]

ElegantOTA.setAuth(ename, epwd);
The reason why there is no Auth now is because the library editor removed those functions and now the cut-down version is free and if one wants auth and all the other stuff like custom UI then
one has to pay. often when these libraries or even the esp32 code is updated its bould to break multiple things.

Sure.. I'm just saying, as it is right now it's not compiling for me

myeota_srvr is a pointer to a class. So its testing to see of the pointer points to a valid instance of a class and so somewhere in the code this pointer will point to an instance of the class, until then, I dont think I need to spell that out.

In setup() line 1761
myeota_srvr = new EOTA_SERVER();

the pointer is assigned to a new instance of a class
Next there is a call to start the new instance via the class pointer myeota_srvr
ota_status = eota_start();

Yes.. I noticed that. But as the code is right now, eota won't start at all.
What happens is:
- Inside setup(), there's the line you mentioned that creates the eota server
- a couple of lines down, you have: ota_status = eota_start();
- entering the eota_start function, the first thing that is checked is whether the server is running, which makes sense:

    if (ota_status == STATUS_RUNNING) {
      return true;
    }

• But immediately afterwards, there's the check line I mentioned:

    // Check if there is an instance of the class
   if (myeota_srvr) {
      return true;
    }

Here, myeota_srv DOES exist, but it's not running yet (as this would happen in eota_start a few lines down). But because there's an early return, the server will never start.

Which is why I proposed to switch the logic instead (if(!myeota_srvr) { return false; }.

As for everything else, yes, switching to config based approach rather than ifdefs makes more sense of course. But that's why I would check for arduino-specific device type defs, rather than the config name define. But no worries, those are much less important points, and very subjective of course.

[brownrb]

Hi Marco
Thanks for the clarification, I can now see my mistake,

In review,

which I had to comment out, as there's no such method in ElegantOTA 2.2.7

and I looked into this further, as I was sure that I had tested OTA before the 317 release. Turns out I had tested the Auth functionality. But the library I am using is ElegantOTA 3.1.6 and I inadvertently gave you the wrong info re auth. I will blame that on old age and dying brain cells. Yes, some features were deleted from the free (lite) version, and the settings I was using (ID) was in fact used in my code but that was fixed earlier.

So all should be okay now, and I think I will just go ahead and bump the revision to 318 with the minor fix, the next planned update is not ready for testing so its best I do it like that for now,

regards
Robert

[Collet]

Hi Robert, all

it appears that there is a similar issue for the text display in display_txt.cpp, line 135

bool TEXT_DISPLAY::start() {

// prevent any attempt to start if already started
if (display_status == STATUS_STOPPED) {
return STATUS_STOPPED;
}

this is an endless loop, the display never start.

the correct test should be:

if (display_status == STATUS_RUNNING) {
return true;
}

which actually is correct for the display_lilygo.cpp and display_graphics.cpp.

Regards
Joël

[brownrb]

Many thanks Joël
I had added that to the 319 release I am currently working on.

regards
Robert