MySensors icon indicating copy to clipboard operation
MySensors copied to clipboard

Simplify and Fix support for esp8266 core v2.7 + v3

Open d-a-v opened this issue 3 years ago • 11 comments

This proposal removes every core overrides that are needed to call _process() and uses schedule_recurrent_function_us() to call it from the pseudo-background.

Not tested.

Aims at fixing #1496

@Yveaux @virtual-maker @dungdao191299 @mfalkvidd @konstruktors.

d-a-v avatar Nov 11 '21 14:11 d-a-v

This change for esp8266 Arduino core v3+ should be also compatible with v2.6 and v2.7.

d-a-v avatar Nov 11 '21 14:11 d-a-v

A mail with a patch has been sent to you that, if applied to your PR, will make it follow the MySensors coding standards.

I can't find this email in my mailbox nor its content from any of the CI links above.

Anyway, this patch is very simple. I'm not looking for credits and maintainer can use/integrate it after checking. Please tell me if this proposal is wrong or if I can close it (or why I can't receive the email - twice, spam checked)

d-a-v avatar Nov 14 '21 12:11 d-a-v

I've tested this patch with the following Platform.io project https://github.com/kasparsd/era/compare/arduino-test with Wemos D1 mini (ESP8266) and it appears to be working as expected now. Here is the output from the build:

> Executing task in folder mysensors-arduino-test: platformio run --target upload --target monitor --environment d1_mini <

Processing d1_mini (framework: arduino; platform: [email protected]; board: d1_mini)
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/espressif8266/d1_mini.html
PLATFORM: Espressif 8266 (3.2.0) > WeMos D1 R2 and mini
HARDWARE: ESP8266 80MHz, 80KB RAM, 4MB Flash
PACKAGES: 
 - framework-arduinoespressif8266 3.30002.0 (3.0.2) 
 - tool-esptool 1.413.0 (4.13) 
 - tool-esptoolpy 1.30000.201119 (3.0.0) 
 - tool-mklittlefs 1.203.210628 (2.3) 
 - tool-mkspiffs 1.200.0 (2.0) 
 - toolchain-xtensa 2.100300.210717 (10.3.0)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 36 compatible libraries
Scanning dependencies...
Dependency Graph
|-- <MySensors> 2.4.0-alpha+sha.5b45fe6
|   |-- <SPI> 1.0
|   |-- <Wire> 1.0
|   |-- <EEPROM> 1.0
|   |-- <ESP8266WiFi> 1.0
Building in release mode
Compiling .pio/build/d1_mini/src/main.cpp.o
In file included from .pio/libdeps/d1_mini/MySensors/MySensors.h:378,
                 from src/main.cpp:9:
In function 'bool _serialProcess()',
    inlined from 'bool _serialProcess()' at .pio/libdeps/d1_mini/MySensors/hal/transport/RS485/MyTransportRS485.cpp:129:6:
.pio/libdeps/d1_mini/MySensors/hal/transport/RS485/MyTransportRS485.cpp:147:10: warning: 'void* memcpy(void*, const void*, size_t)' accessing 5 bytes at offsets 0 and 1 overlaps 4 bytes at offset 1 [-Wrestrict]
  147 |    memcpy(&_header[0],&_header[1],5);
      |    ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
Linking .pio/build/d1_mini/firmware.elf
Retrieving maximum program size .pio/build/d1_mini/firmware.elf
Checking size .pio/build/d1_mini/firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [===       ]  35.0% (used 28660 bytes from 81920 bytes)
Flash: [===       ]  26.4% (used 276177 bytes from 1044464 bytes)
Building .pio/build/d1_mini/firmware.bin
Creating BIN file ".pio/build/d1_mini/firmware.bin" using "/Users/.../.platformio/packages/framework-arduinoespressif8266/bootloaders/eboot/eboot.elf" and ".pio/build/d1_mini/firmware.elf"
Configuring upload protocol...
AVAILABLE: espota, esptool
CURRENT: upload_protocol = esptool
Looking for upload port...
Auto-detected: /dev/cu.usbserial-1420
Uploading .pio/build/d1_mini/firmware.bin
esptool.py v3.0
Serial port /dev/cu.usbserial-1420
Connecting....
Chip is ESP8266EX
Features: WiFi
Crystal is 26MHz
MAC: xx:xx:xx...
Uploading stub...
Running stub...
Stub running...
Configuring flash size...
Compressed 280336 bytes to 205154...
Writing at 0x00000000... (7 %)
Writing at 0x00004000... (15 %)
Writing at 0x00008000... (23 %)
Writing at 0x0000c000... (30 %)
Writing at 0x00010000... (38 %)
Writing at 0x00014000... (46 %)
Writing at 0x00018000... (53 %)
Writing at 0x0001c000... (61 %)
Writing at 0x00020000... (69 %)
Writing at 0x00024000... (76 %)
Writing at 0x00028000... (84 %)
Writing at 0x0002c000... (92 %)
Writing at 0x00030000... (100 %)
Wrote 280336 bytes (205154 compressed) at 0x00000000 in 19.9 seconds (effective 112.6 kbit/s)...
Hash of data verified.

Leaving...
Hard resetting via RTS pin...
========================================================================== [SUCCESS] Took 27.19 seconds ==========================================================================
--- Available filters and text transformations: colorize, debug, default, direct, esp8266_exception_decoder, hexlify, log2file, nocontrol, printable, send_on_enter, time
--- More details at https://bit.ly/pio-monitor-filters
--- Miniterm on /dev/cu.usbserial-1420  9600,8,N,1 ---
--- Quit: Ctrl+C | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---
5097 !MCO:SND:NODE NOT REG
10125 !MCO:SND:NODE NOT REG
15154 !MCO:SND:NODE NOT REG
20183 !MCO:SND:NODE NOT REG
25213 !MCO:SND:NODE NOT REG
30242 !MCO:SND:NODE NOT REG
...

which previously wouldn't show up at all.

kasparsd avatar Nov 21 '21 10:11 kasparsd

Are there any remaining things to be adjusted for this to get merged?

kasparsd avatar Apr 10 '22 09:04 kasparsd

If CI must pass before merging, someone may trigger it for a new run/test or help me fix as I don't understand the detailed issue.

Or I'm also fine if a maintainer independently applies this three-lines patch to the main branch.

d-a-v avatar Apr 11 '22 11:04 d-a-v

@d-a-v I don't have any additional insight but it looks like the failing test is reporting an issue with Arduino esp8266 core 2.7.4:

.arduino15/packages/esp8266/hardware/esp8266/2.7.4/cores/esp8266/core_esp8266_main.cpp:185: undefined reference to `loop'

during:

Compiler warnings/errors:
/var/lib/jenkins/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/2.5.0-4-b40a506/bin/../lib/gcc/xtensa-lx106-elf/4.8.2/../../../../xtensa-lx106-elf/bin/ld: /var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1513/workspace/build/core/core.a(core_esp8266_main.cpp.o): in function `__loop_end':
/var/lib/jenkins/.arduino15/packages/esp8266/hardware/esp8266/2.7.4/cores/esp8266/core_esp8266_main.cpp:185: undefined reference to `setup'
/var/lib/jenkins/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/2.5.0-4-b40a506/bin/../lib/gcc/xtensa-lx106-elf/4.8.2/../../../../xtensa-lx106-elf/bin/ld: /var/lib/jenkins/.arduino15/packages/esp8266/hardware/esp8266/2.7.4/cores/esp8266/core_esp8266_main.cpp:185: undefined reference to `loop'
/var/lib/jenkins/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/2.5.0-4-b40a506/bin/../lib/gcc/xtensa-lx106-elf/4.8.2/../../../../xtensa-lx106-elf/bin/ld: /var/lib/jenkins/jobs/MySensors/jobs/MySensors/branches/PR-1513/workspace/build/core/core.a(core_esp8266_main.cpp.o): in function `loop_wrapper()':
/var/lib/jenkins/.arduino15/packages/esp8266/hardware/esp8266/2.7.4/cores/esp8266/core_esp8266_main.cpp:191: undefined reference to `setup'
/var/lib/jenkins/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/2.5.0-4-b40a506/bin/../lib/gcc/xtensa-lx106-elf/4.8.2/../../../../xtensa-lx106-elf/bin/ld: /var/lib/jenkins/.arduino15/packages/esp8266/hardware/esp8266/2.7.4/cores/esp8266/core_esp8266_main.cpp:192: undefined reference to `loop'
collect2: error: ld returned 1 exit status
exit status 1

which is triggered when trying to build this:

Building MySensors/tests/Arduino/sketches/hard_signing_no_whitelisting_full_debug/hard_signing_no_whitelisting_full_debug.ino using /opt/arduino-1.8.13/arduino-builder -warnings=all -hardware /opt/arduino-1.8.13/hardware -tools /opt/arduino-1.8.13/hardware/tools/avr -tools /opt/arduino-1.8.13/tools-builder -built-in-libraries /opt/arduino-1.8.13/libraries -hardware /var/lib/jenkins/.arduino15/packages -tools /var/lib/jenkins/.arduino15/packages -hardware hardware -libraries .  -build-path build -fqbn=esp8266:esp8266:generic:xtal=80,vt=flash,exception=disabled,ResetMethod=ck,CrystalFreq=26,FlashFreq=40,FlashMode=dout,eesz=512K,led=2,ip=lm2f,dbg=Disabled,lvl=None____,wipe=none,baud=115200

You mentioned that:

It should work with v2.6+ (v2.6.x, v2.7.x, v3.0.x) of the esp8266 arduino core. This API was not present in v2.5.x.

so I'm not sure why it would be failing there.

kasparsd avatar Apr 11 '22 15:04 kasparsd

I honestly don't know what happens

You are linking to the second CI error which is indeed weird, because the error is undefined reference to 'loop' and undefined reference to 'setup' which seems fine because the source sketch does not have them (while library examples do define them).

d-a-v avatar Apr 11 '22 17:04 d-a-v

Sorry for the long wait.

I think https://github.com/mysensors/MySensors/pull/1524 provides a solution to the same problem. Since 1524 has been merged, I'm closing this PR. Feel free to reopen with a comment if you think otherwise.

mfalkvidd avatar Jul 08 '22 13:07 mfalkvidd

@virtual-maker

Compiling .pioenvs/mysensors/libc02/EEPROM/EEPROM.cpp.o
In file included from .piolibdeps/mysensors/MySensors/hal/architecture/ESP8266/MyMainESP8266.cpp:41,
                 from .piolibdeps/mysensors/MySensors/MySensors.h:453,
                 from src/ESPHomeMySensorsGatewayShim.h:9,
                 from src/main.cpp:25:
/Users/chytreg/.platformio/packages/framework-arduinoespressif8266/cores/esp8266/core_esp8266_main.cpp:141:27: error: macro "yield" passed 1 arguments, but takes just 0
  141 | extern "C" void yield(void) __attribute__ ((weak, alias("__yield")));
      |                           ^
src/main.cpp:20: note: macro "yield" defined here
   20 | #define yield() esphome::yield()
      |
Compiling .pioenvs/mysensors/lib071/SPI/SPI.cpp.o
Compiling .pioenvs/mysensors/libf4d/MySensors/MyASM.S.o
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/BearSSLHelpers.cpp.o
Archiving .pioenvs/mysensors/libf4d/libMySensors.a
Indexing .pioenvs/mysensors/libf4d/libMySensors.a
Archiving .pioenvs/mysensors/libc02/libEEPROM.a
Indexing .pioenvs/mysensors/libc02/libEEPROM.a
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/CertStoreBearSSL.cpp.o
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/ESP8266WiFi.cpp.o
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/ESP8266WiFiAP.cpp.o
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/ESP8266WiFiGeneric.cpp.o
Compiling .pioenvs/mysensors/libac2/ESP8266WiFi/ESP8266WiFiGratuitous.cpp.o
In file included from .piolibdeps/mysensors/MySensors/hal/architecture/ESP8266/MyMainESP8266.cpp:41,
                 from .piolibdeps/mysensors/MySensors/MySensors.h:453,
                 from src/ESPHomeMySensorsGatewayShim.h:9,
                 from src/main.cpp:25:
/Users/chytreg/.platformio/packages/framework-arduinoespressif8266/cores/esp8266/core_esp8266_main.cpp:141:17: error: variable or field 'yield' declared void
  141 | extern "C" void yield(void) __attribute__ ((weak, alias("__yield")));
      |                 ^~~~~

I'm getting this error when I try compile against mysensors/MySensors#development The code compiles from d-a-v/MySensors#76f2545e7f9693af9ce8e0539b81dda93620beea cc @d-a-v

Here is a gist with details: https://gist.github.com/chytreg/31fb06a72bb1c964c631f1241af4951b It's strange, because GitHub shows only a little changes in diff.

dgertych-monterail avatar Sep 27 '22 09:09 dgertych-monterail

@chytreg I found this in your gist listings:

src/main.cpp:20: note: macro "yield" defined here
   20 | #define yield() esphome::yield()

Can you provide this main.cpp file in gist and post here file path in your project? Thank you

ATM I'm argue that you use some kind of ESPHome framework and try to include MySensors into it. Unfortunately this ESPFramework brings in it's own Main.cpp file and don't use the ESP8266 core main file core_esp8266_main.cpp.

I'm afraid this combination of ESPHome and MySensors will not work because both introduce and include special Main() functions.

virtual-maker avatar Sep 28 '22 21:09 virtual-maker

I've added main.cpp to the gist, it's generated by esphome. The original version of this code should work according o the author https://github.com/mannkind/ESPHomeMySensorsGatewayShim on

Tested with ESPHome 1.13.x, MySensors 2.3.2, and a Wemos D1 Mini w/an RF24 Radio.

Im trying to compile it against recent version of ESPHome which have upgraded espressif8266 version since then.

dgertych-monterail avatar Sep 28 '22 22:09 dgertych-monterail