fredlcore / BSB-LAN

LAN/WiFi interface for Boiler-System-Bus (BSB) and Local Process Bus (LPB) and Punkt-zu-Punkt Schnittstelle (PPS) with a Siemens® controller used by Elco®, Brötje® and similar heating systems
225 stars 84 forks source link

Compiling for ESP32 under Arduino IDE fails in several points #322

Closed fredlcore closed 3 years ago

fredlcore commented 3 years ago

Compiling the source code for the ESP32 architecture under PlatformIO / VSCode works fine, but not under Arduino IDE. Apart from some libraries that have to be removed/zipped because they are incompatible with ESP32, the main issue lies with this error:

/tmp/arduino_build_761330/sketch/BSB_lan.ino.cpp -o /tmp/arduino_build_761330/sketch/BSB_lan.ino.cpp.o
BSB_lan:6279:23: error: conflicting declaration of 'void loop()' with 'C' linkage
In file included from /tmp/arduino_build_761330/sketch/src/BSB/bsb.h:6:0,
                 from /home/db/BSB_lan/BSB_lan.ino:439:
/home/db/.arduino15/packages/esp32/hardware/esp32/1.0.4/cores/esp32/Arduino.h:119:6: note: previous declaration with 'C++' linkage
 void loop(void);
      ^
(...)
exit status 1
conflicting declaration of 'void loop()' with 'C' linkage

The Arduino library pre-declares the loop() and setup() functions there, so that should actually not be a problem. Commenting out the loop() line allows compilation to proceed, but at the end the compiler fails because the loop-function was not predeclared, so it's a catch-22 situation.

I think it could have something to do with the way the Arduino.h library is being included. Some libraries include it with quotes (" ") and some with brackets (< >). The former tries to load it from the working directory, the other one from the system library folder. It could be that with PlatformIO, these are the same and with the Arduino IDE, they are not, for whatever reason.

A search for Arduino.h on the Arduino IDE system would probably be a good start to see where the library is located and whether there are any differences. But at least on my system, it all looks good. The only weird occurrence is an Arduino.h in the tests-subfolder of the PubSubClient library. But there should not be a way that this would be included.

One difference I can see between the AVR and the ESP32 Arduino.h is that both have these lines:

#ifdef __cplusplus
extern "C" {
#endif

but the ESP32 version does this only after pre-declaring loop() and setup(), whereas the AVR version has these functions inside the extern "C" block. Not sure if this is relevant, though.

@do13 and @dukess, do you have any other ideas?

1coderookie commented 3 years ago

Ok, ESP32 wasn't accessable anymore, so I tried to re-flash the version from yesterday - seems that changing the settings via webconfig killed something:

esptool.py v2.6
Serial port /dev/ttyUSB0
Connecting....
Chip is ESP32D0WDQ6 (revision 1)
Features: WiFi, BT, Dual Core, 240MHz, VRef calibration in efuse, Coding Scheme None
MAC: 4c:11:ae:9b:dd:f0
Uploading stub...
Running stub...
Stub running...
Changing baud rate to 921600
Changed.
Configuring flash size...

A fatal error occurred: Invalid head of packet (0x2C)
A fatal error occurred: Invalid head of packet (0x2C)

Then I noticed that the upload speed wasn't set to 115200 - I changed it and right now it's flashing.

fredlcore commented 3 years ago

Please check for error messages in serial monitor and load configuration via /C, otherwise they just stay in memory.

1coderookie commented 3 years ago

..I'll send you the stuff via email, don't wanna spam the issue.. ;)

dukess commented 3 years ago

I see no other way than to get back to the I2C EEPROM. In the end, it's just 30 cents or so of extra cost.

At this time is better to stay with I2C EEPROM

We can reduce numbers of parameters that require reboots after configuration changes but then we need handlers for applying new values on the fly.

fredlcore commented 3 years ago

Ah, I didn't know this! So there is a regular reboot after saving configuration? Then that's fine, I just thought that's a crash followed by a reboot.

1coderookie commented 3 years ago

Yes, at least certain settings require and initiate a reboot like I mentioned above. But the reboot didn't seem to be 100% successful as I reported to you via email, but let's test and see..

fredlcore commented 3 years ago

That was my understanding, too, that certain settings require a reboot - and I thought that only in that cases a reboot is initiated. So for simply changing log parameters or something like that, I thought that there is no reboot. But I stand corrected, and there's no need right now to change that.

1coderookie commented 3 years ago

Yes, you're right - also from my perspective a reboot doesn't always occur - at least with the Due. -> Is that correct @dukess? It seemed to occur with the ESP though, as I mentioned in the edit above. But we'll test, compare and see, once the ESP runs again..

dukess commented 3 years ago

So for simply changing log parameters or something like that, I thought that there is no reboot.

These parameters not needed to reboot. Not need reboot now: bus type, addresses, PPS mode RGT emulation (sensors) averages, log list, intervals, etc. MAX settings MQTT settings

Do reboot: Network parameters: MAC. DHCP mode, IP address, etc. RGT emulation (buttons settings)

Update Need to add reboot when OneWire pin will change.. Miss it: already done.

fredlcore commented 3 years ago

Thanks for the clarification!

fredlcore commented 3 years ago

As for the I2CEEPROM and the ESP32: I have tested it here and it works quite well, I only get this error (or is it just a warning?) a couple of times during the EEPROM dump for example: [E][esp32-hal-i2c.c:1426] i2cCheckLineState(): Bus Invalid State, TwoWire() Can't init sda=1, scl=1 Some sources say that if this happens continuously, then the internal pullups on the I2C pins are not sufficient. Other sources say that if they just occur occasionally, it's a sign that the I2C bus is busy. What I didn't figure out is if there will be some retries in the case of "bus busy". If there is, I don't think we have a problem here.

Just for my understanding, @dukess: What would happen if a corrupted value would be written to the EEPROM (or skipped), for example from one of the configuration entries? The CRC would not match then, correct? And what happens then? And do the EEPROM functions evaluate the return codes of read and write functions and if there is an error like "bus busy" try again? Maybe that would be helpful?

dukess commented 3 years ago

CRC will calculate for scheme structure (source data is options addresses in EEPROM) not options values. I thinking about calculation CRC for values but i left this idea, because i thought that wear of the cell with CRC value would be too fast (every time any value changes).

dukess commented 3 years ago

@fredlcore When you did this hack

EEPROMClass EEPROM_ESP("eeprom1", 0x1000);
#define EEPROM EEPROM_ESP

settings was saved/loaded without problem?

dukess commented 3 years ago

Other way we can do is make Issue/PR in https://github.com/espressif/arduino-esp32/

dukess commented 3 years ago

I read the EEPROM library source codes and realized that we were doing something wrong: EEPROM.begin(EEPROM_SIZE); // this function init EEPROM, set EEPROM size and read EEPROM into internal buffer. EEPROM.write(addr, val); //Read byte from buffer (not EEPROM) EEPROM.write(addr, val); //Write byte to buffer EEPROM.commit(); //flush data from buffer to EEPROM. EEPROM.end(); //finish. I think we not need it.

Update. I'm dumb. I saw it all in previous commits.

fredlcore commented 3 years ago

Thanks for the CRC explanation, that makes total sense not to calculate the CRC across all values, we have to minimize writing to an absolute minimum of course. As for the "hack", that was for the internal EEPROM, not the I2CEEPROM. The warning mentioned above is only coming from the I2C Library (Wire.h). And it's not even showing any (visible) problems now, but I just want to make sure that it's generally not a problem. And as I have mentioned here #329, this is probably because of the insufficient internal pull-ups in the ESP32.

As for the "hack" and the internal EEPROM, I generally had problems / error messages because apparently the (default?) partition map that I'm using with my ESP32 has only (the standard?) 512 bytes of flash reserved for the EEPROM. So that's why as mentioned in #329, I would like to check if there is another way to use the ESP32's internal EEPROM in a way that also novice users / noobs can use this setup.

An alternative would be to check if we can use SPIFFS on the ESP32 and make these look like EEPROM calls. Or just use ifdefs because the actual commands writing to EEPROM are just used in couple of lines.

1coderookie commented 3 years ago

Maybe it's nonsense, but I read about the need to erase the "EEPROM"-sector prior to writing something new, I just mentioned it here: https://github.com/fredlcore/bsb_lan/issues/329#issuecomment-773147078

dukess commented 3 years ago

Can you send a more complete system startup log with internal EEPROM initialization, as well as reading/writing parameters? I hope to find error/warning messages from EEPROM.

fredlcore commented 3 years ago

@dukess: Sure, but let's continue the ESP32-EEPROM related discussion here: #329

fredlcore commented 3 years ago

I now have the problem that the current version freezes on the Due right after "READY" on my two Dues. Could anyone of you confirm this and try an earlier version from a day or two ago, so we can figure out what causes the problem? I'm on the road until tonight and can't test right now...

1coderookie commented 3 years ago

Confirmed: 16:49:36.560 -> READY - that's it.. Version from yesterday (which worked fine at Due yesterday) now also doesn't work anymore, same output like above! Initiated EEPROM clearing through pins, but that also seems to freeze - no reaction anymore after 16:55:06.255 -> Clearing EEPROM... since 3min and the LED is still lit.. Reboot didn't change anything, freezes after "READY". Hope we didn't kill anything now / brick it.. :(

1coderookie commented 3 years ago

..maybe better to put the version from yesterday or the day before yesterday up on GitHub again right now - not that anyone downloads it right now and maybe kills something.. :(

fredlcore commented 3 years ago

No, nothing is killed. So it still gets to the point where you can erase the EEPROM?

It reminds me of the bug where the second line printed to serial would freeze the Arduino, it was some kind of buffet overflow, but not sure if/where this is the case here.

fredlcore commented 3 years ago

For a quick check you could try version 1.1 from releases...

dukess commented 3 years ago

Try to replace

  uint8_t empty_block[4097] = { 0xFF };
  EEPROM.fastBlockWrite(0, &empty_block, 4096);

with

uint8_t empty_block[4096] = { 0xFF };
 EEPROM.fastBlockWrite(0, empty_block, 4096);

&empty_block - we get address of variable empty_block - get address of array

1coderookie commented 3 years ago

Well, I just connected the two pins and pressed reboot to erase EEPROM, after that how I described above.

@dukess: Did it with the current version right now - didn't change anything: 17:54:44.054 -> READY

@fredlcore: Trying to flash v1.1 now.

1coderookie commented 3 years ago

v1.1 works.

dukess commented 3 years ago

Do we need to replace pinMode(19, INPUT); //AFAIK RX-pin for hardware serial on Due

with pinMode(temp_bus_pins[0], INPUT);

? And moving below, sure.

dukess commented 3 years ago

My eyes don't hang to anything anymore.

fredlcore commented 3 years ago

I did this:

Serial.println(3.8);
  readFromEEPROM(CF_PPS_VALUES);
Serial.println(3.9);

and 3.8 gets still printed, but 3.9 no more. Does that ring a bell, @dukess?

dukess commented 3 years ago

I would be to try comment this string. If sketch hangs on next EEPROM reading then i will looking on list compiled-in libraries.

fredlcore commented 3 years ago

It hangs on whatever is the first readFromEEPROM(), and to be more exact it's the EEPROM.read() that hangs. That's strange, because if the EEPROM wasn't ready or present, if (!EEPROM_ready) return false;should have returned false already.

dukess commented 3 years ago

It is correct definition: #if defined(ARM) ? or it should be defined(__arm__) ?

dukess commented 3 years ago

Answering myself: should be #if defined(__arm__) I tried to compile software and compiled size increased with this definition.

fredlcore commented 3 years ago

Saviour of the day :)!

fredlcore commented 3 years ago

Do we need to replace pinMode(19, INPUT); //AFAIK RX-pin for hardware serial on Due

with pinMode(temp_bus_pins[0], INPUT);

? And moving below, sure.

I have to get better in commenting my own code :(. I'm not sure why I put this in there, maybe it was to turn off the internal pullups, so this could be a Due-(i.e. ARM-)specific setting. Since it seems not to be necessary for the ESP32, we won't have to move it down. Also, we don't want it to be active for the SoftwareSerial pin, so that's why temp_bus_pins[0] won't necessarily be good. I guess it's safest to leave it as it is for now until I (or someone else) remembers why we put it there ;)...

dukess commented 3 years ago

But if I move this line to line 9378, nothing terrible will happen? :-) (And put it in the definition:

  #if defined(__arm__) || defined(ESP32)
  pinMode(temp_bus_pins[0], INPUT); //RX-pin of hardware serial on Due/ESP32
  #endif

)

fredlcore commented 3 years ago

I think the safest would be to make it a general command and test it with all three systems. Because I'm not sure if it would also be necessary on Mega systems which use HardwareSerial...

fredlcore commented 3 years ago

I'll close this for now because it looks like the ESP32 build works mostly stable now. We can reopen it if further problems arise. Thanks again to everybody for ideas and testing!