Saur0o0n / PIDKiln

Kiln PID controller based on Espressif Systems ESP32 chip board with Arduino IDE.
GNU General Public License v2.0
102 stars 38 forks source link

Big bug #10

Closed Beethoven56 closed 2 years ago

Beethoven56 commented 2 years ago

Hello Adrian, First off all: What a fantastic piece of software. Just what i was looking for. I also had problems with a reboot loop even with a ESP32-WROVER come to find out that the reason is a bug in the software.

In module PIDKiln_program Line 149. If the "/" character is not found, fname will become 0 and the program will crash and reboot in line 152. The same happens in module PIDKiln_log in line 158

char* fname; char tmp[32]; uint8_t len2;

strcpy(tmp,file.name()); len2=strlen(tmp); if(len2>31 || len2<2) return 2; // file name with dir too long or just / fname=strchr(tmp+1,'/'); // seek for the NEXT directory separator... fname++; // ..and skip it if(!strcmp(fname,"index.html")) continue; // skip index file strcpy(Programs_DIR[Programs_DIR_size].filename,fname); Programs_DIR[Programs_DIR_size].filesize=file.size(); Programs_DIR[Programs_DIR_size].sel=0;

I could get it to work by modifying the code quick and dirty as i am not good in C++

if (strchr(tmp+1,'/')!= 0) { fname=strchr(tmp+1,'/'); // seek for the NEXT directory separator... fname++; // ..and skip it } if(!strcmp(tmp,"index.html")) continue; // skip index file strcpy(Programs_DIR[Programs_DIR_size].filename,tmp); Programs_DIR[Programs_DIR_size].filesize=file.size(); Programs_DIR[Programs_DIR_size].sel=0;

I would be nice if you could have a look at it and do it the right way.

Saur0o0n commented 2 years ago

Hi, What was your "/programs" directory structure that it failed? Was it empty?

Beethoven56 commented 2 years ago

No. It's just filled with the testfiles which came with the program. No subdir.

Saur0o0n commented 2 years ago

It's definitely not pretty part of the code - but whole esp32 disk support is rather clumsy, so I did not care. To be honest, I don't see how it could fail. This part of the code is looking for PRG_DIRECTORY="/programs", so every listed file should be like this "/programs/program1.txt", "/programs/program2.txt" and so one. So there has to be next / after we skip the first one.

If you can, please run debug with this additional line - we will see, when (a probably why) it's failing

    if(len2>31 || len2<2) return 2; // file name with dir too long or just /
    DBG dbgLog(LOG_DEBUG,"[PRG] Processing filename: %s\n",tmp);
    fname=strchr(tmp+1,'/');        // seek for the NEXT directory separator...
    fname++;                        //  ..and skip it
    if(!strcmp(fname,"index.html")) continue;   // skip index file
Beethoven56 commented 2 years ago

Here the result:

02:32:20.189 -> [PRG] Loading directory... 02:32:20.242 -> [PRG] counted 6 files 02:32:20.342 -> [PRG] Processing filename: program1.txt 02:32:20.342 -> Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled. 02:32:20.342 -> 02:32:20.342 -> Core 1 register dump: 02:32:20.342 -> PC : 0x4008b67f PS : 0x00060630 A0 : 0x800dc81a A1 : 0x3ffb2620
02:32:20.342 -> A2 : 0x00000001 A3 : 0x3f4005c8 A4 : 0x3ffe30b8 A5 : 0x00000029
02:32:20.342 -> A6 : 0x0000001e A7 : 0x3ffc2880 A8 : 0x00000000 A9 : 0x3ffb25b0
02:32:20.389 -> A10 : 0x3ffe30b8 A11 : 0x3ffe30b8 A12 : 0x3f4015e8 A13 : 0x3ffb2610
02:32:20.389 -> A14 : 0x3ffb25e0 A15 : 0x00000008 SAR : 0x00000007 EXCCAUSE: 0x0000001c
02:32:20.389 -> EXCVADDR: 0x00000001 LBEG : 0x400fe9e4 LEND : 0x400fe9ee LCOUNT : 0x00000000
02:32:20.389 -> 02:32:20.389 -> 02:32:20.389 -> Backtrace:0x4008b67c:0x3ffb26200x400dc817:0x3ffb2630 0x400dcb2e:0x3ffb26e0 0x400dd938:0x3ffb27d0 0x400f2777:0x3ffb2820 02:32:20.389 -> 02:32:20.389 -> 02:32:20.389 -> 02:32:20.389 -> 02:32:20.389 -> ELF file SHA256: 0000000000000000 02:32:20.389 -> 02:32:20.389 -> Rebooting...

Saur0o0n commented 2 years ago

Hi and thanks. So like I said - this should be "/programs/program1.txt" - not just "program1.txt". I'll have to check, assuming your directory structure is ok, perhaps something has change in SPIFS handling in newer esp32 platform package. I wouldn't be surprise, since this was a mess with a lot of errors when I wrote the code.

Saur0o0n commented 2 years ago

So I've confirmed that somewhere with esp32 2.0 CI, SPIFFS handling was "fixed" (long time ago I've put bug report about it - ignored of course) - this changes directory handling. Previously it was like this:

Writing file: /test/hello1.txt
- file written
Writing file: /test/hello2.txt
- file written
 Listing / directory 
Listing directory: /
  FILE: /hello1.txt SIZE: 6
  FILE: /hello2.txt SIZE: 6
  FILE: /test/hello1.txt    SIZE: 6
  FILE: /test/hello2.txt    SIZE: 6
> Listing /test directory 
Listing directory: /test
  FILE: /test/hello1.txt    SIZE: 6
  FILE: /test/hello2.txt    SIZE: 6
> Listing /test/ directory 
Listing directory: /test/
> Test complete

Now it's like this:

Writing file: /test/hello1.txt
- file written
Writing file: /test/hello2.txt
- file written
 Listing / directory 
Listing directory: /
  FILE: hello1.txt  SIZE: 6
  FILE: hello2.txt  SIZE: 6
  FILE: hello1.txt  SIZE: 6
  FILE: hello2.txt  SIZE: 6
> Listing /test directory 
Listing directory: /test
  FILE: hello1.txt  SIZE: 6
  FILE: hello2.txt  SIZE: 6
> Listing /test/ directory 
Listing directory: /test/
> Test complete

Anyway - have to fix it now in pidkiln. Sad thing is, I don't find any mention of it in bugfixes of ESP32 Arduino core and it's still totally buggy - it wont list directory ending with / and don't list directories in main dir :( (see above)

Anyway, a lot of changes has to be made...

Saur0o0n commented 2 years ago

Ok, I've fixed the issue with new SPIFFS handler for ESP32 2.0.x framework, I've also made it easier to use it on Wrover modules and fixed some stack issues. I'll probably push tomorrow 1.2 version, but have to do more testing.

Saur0o0n commented 2 years ago

Pushed v1.2 release that fixed this and other issues.