Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ESP8266 for Octopus PRO v1.1/H723 #26992

Closed

Conversation

dc740
Copy link
Contributor

@dc740 dc740 commented Apr 21, 2024

Description

Add ESP8266 support for the Octopus PRO board. I tested it on the H723 version.

From what I could see in the BTT store, only the ESP-07S is sold for the Octopus Pro, however, the ESP8266MOD from the picture can be used as-is by making sure GPIO15 is properly set for the normal boot mode. Of course we are using it from the serial port instead of SPI as other firmwares, but it's still very handy.

Boot modes are defined by setting GPIO0,GPIO2 and GPIO15. The thing is, SOME boards don't need GPIO15, but others do. Which I assume is the reason not many people hit this bug. (more details below)
This is a board I had from many years ago:
immagine

Turns out the pinout is compatible with ESP 07S and ESP12, but these must have something different than the barebones 8266 from the picture, because the BTT store sells it as supported while the 8266 is listed for the SKR 2 only. The board is currently working as expected after my changes though.

Regarding boot modes.

(I found this on the internet, I didn't read the datasheet.)
Considering GPIO[0-2-15], then:
0-1-0: flash mode (for programming it)
1-1-0: Normal boot from flash (the mode we want)
0-0-1: boot from SD

The bug is that GPIO15 is not LOW, so the board hangs in the bootloader "waiting for the host".
After making these changes I was able to connect to the web server and the display shows the output from the ESP commands (like the IP it has been assigned)

Regarding the features.ini change:

The issue is that we need WIFISUPPORT in order to run the pins setup and restart the ESP8266, but it was trying to compile a lot of libraries that (AFAIK) are only required when you use the actual ESP as the main system. I was able to compile the firmware by removing the unneeded dependencies.

Requirements

ESP8266.

Benefits

Adds support for the ESP8266 on the Octopus Pro v1.1

Configurations

Just enable WIFISUPPORT

Related Issues

None that I could find.

@sjasonsmith sjasonsmith added the Needs: Work More work is needed label Apr 21, 2024
@dc740
Copy link
Contributor Author

dc740 commented Apr 21, 2024

I see the test pipeline fails because it's expecting the ESP32 when enabling the wifi. In this case what I'm doing is to run Marlin in the BTT Octopus PRO and using SERIAL_2 to communicate over wifi with ESP3D (version 2)
IMG20240421195958

(Excuse the lack of a proper pcb. I just had the module around. Put some double sided tape, and soldered the pins to on a proto board to simulate MKS or BTT style esp board)
Here is the configuration:
Configuration.zip

I need to run the esp_wifi init method, but I won't be using OTA updates nor web support, nor esp3dlib custom commands parser.
I would like to enable just wifi for file transferring (although slow), and an easier way to control de printer than a rotary encoder, so adding the wifi module on top of the octopus is more than enough for my needs, but it seems it's always assumed to be running esp3dlib?

@dc740 dc740 changed the title Fix ESP8266 for Octopus PRO 1.1/H723 Fix ESP8266 for Octopus PRO v1.1/H723 Apr 21, 2024
@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 21, 2024

it's always assumed to be running esp3dlib?

Yes. You’d normally flash these modules with ESP3D so they’d run their own firmware. Then you’d enable the associated serial port and Marlin would communicate via serial without needing to enable WIFISUPPORT or ESP3D_WIFISUPPORT options in the config.

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 21, 2024

Also, please don't break ESP32 WiFi / ESP3DLib support 😄 I don't think Luc could deal with us breaking support again since I just fixed it in #26822.

@dc740
Copy link
Contributor Author

dc740 commented Apr 21, 2024 via email

@sjasonsmith
Copy link
Contributor

@thisiskeithb there are an awful lot of non-ESP32 pins files which include ESP_WIFI_MODULE, which makes it seem like this isn't intended to be an ESP32-only feature.

So what is the state of things then? WIFISUPPORT used to work (meaning perform the various pin initializations in esp_wifi.cpp, but that pre-existing support has been broken by the inclusion of ESP32 libraries in non-ESP32 environments?

Has that been broken since this commit way back in 2020 that moved those library inclusions from the esp32 environment to the *WIFISUPPORT features?
6027055

There was another commit just a few weeks before that which clearly shows it was intended to compile and do things on non-ESP32 environments. The comments from the PR for this mention it fixing issues on the SKR Pro.
8f3d176

Well this just gets more and more interesting the deeper you look.
When the esp_wifi_init was originally added (f5d809f), it didn't require any configuration options at all. It was always executed, but only actually did anything if pins were defined. Now that WIFISUPPORT has to be enabled, we get the unfortunate coupling with the ESP32 library requirements.

@ellensp has already logged information about this issue in #26624...

@dc740
Copy link
Contributor Author

dc740 commented Apr 22, 2024

OK, it seems this is way bigger than I expected. Lets keep this MR open, and continue the discussion in #26624

@sjasonsmith sjasonsmith added the Needs: Discussion Discussion is needed label Apr 22, 2024
@thinkyhead
Copy link
Member

The net change in this PR is now limited to defining a ESP_WIFI_MODULE_GPIO15 pin for boards based on BTT OCTOPUS PRO V1. The pin "ESP-CS" is already shown as PB12 in the port diagram in the pins file, so it appears to be a standard thing. If a standard (and less bodgy) "ESP32" add-on module is used, is this pin usually not connected to anything? If so, then there is no harm to regular WiFi modules with this PR as it is now.

@sjasonsmith
Copy link
Contributor

Just adding support for this pin does not solve the underlying issue, which is that it tries to pull ESP32 dependencies into non-ESP32 builds.

@thisiskeithb
Copy link
Member

Just adding support for this pin does not solve the underlying issue, which is that it tries to pull ESP32 dependencies into non-ESP32 builds.

… when WIFISUPPORT is enabled.

That shouldn’t be enabled since you’re expected to flash the ESP3D firmware to the Wi-Fi module directly and it’ll communicate with Marlin over an enabled serial port.

@sjasonsmith
Copy link
Contributor

Just adding support for this pin does not solve the underlying issue, which is that it tries to pull ESP32 dependencies into non-ESP32 builds.

… when WIFISUPPORT is enabled.

That shouldn’t be enabled since you’re expected to flash the ESP3D firmware to the Wi-Fi module directly and it’ll communicate with Marlin over an enabled serial port.

This entire init function is inside an #if ENABLED(WIFISUPPORT) block. That used to not be the case, I referenced that change in my earlier comment, I think.

@thisiskeithb
Copy link
Member

This goes back to the entire/main issue of what is "Wi-Fi Support" outlined in #26624.

@dc740
Copy link
Contributor Author

dc740 commented Apr 24, 2024 via email

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 24, 2024

I’ve used BTT’s Wi-Fi module (labeled ESP8266 on the PCB and ESP-12S on the metal shield on the top) flashed with ESP3D on an Octopus board without an issue. The only thing I had to enable was a serial port and ESP3D on the Wi-Fi module could communicate just fine.

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 24, 2024

Using current bugfix-2.1.x as of 5366362 (without this PR), BTT's ESP8266/ESP12S module flashed with ESP3D works fine on an Octopus Pro V1.0.

minimum diff without WIFISUPPORT:

diff --git a/Marlin/Configuration.h b/Marlin/Configuration.h
index 72ec272..4e479ed 100644
--- a/Marlin/Configuration.h
+++ b/Marlin/Configuration.h
@@ -68,7 +68,7 @@
 
 // Choose the name from boards.h that matches your setup
 #ifndef MOTHERBOARD
-  #define MOTHERBOARD BOARD_RAMPS_14_EFB
+  #define MOTHERBOARD BOARD_BTT_OCTOPUS_PRO_V1_0
 #endif
 
 /**
@@ -79,7 +79,7 @@
  *
  * :[-1, 0, 1, 2, 3, 4, 5, 6, 7]
  */
-#define SERIAL_PORT 0
+#define SERIAL_PORT -1
 
 /**
  * Serial Port Baud Rate
@@ -92,7 +92,7 @@
  *
  * :[2400, 9600, 19200, 38400, 57600, 115200, 250000, 500000, 1000000]
  */
-#define BAUDRATE 250000
+#define BAUDRATE 115200
 
 //#define BAUD_RATE_GCODE     // Enable G-code M575 to set the baud rate
 
@@ -101,7 +101,7 @@
  * Currently Ethernet (-2) is only supported on Teensy 4.1 boards.
  * :[-2, -1, 0, 1, 2, 3, 4, 5, 6, 7]
  */
-//#define SERIAL_PORT_2 -1
+#define SERIAL_PORT_2 3
 //#define BAUDRATE_2 250000   // :[2400, 9600, 19200, 38400, 57600, 115200, 250000, 500000, 1000000] Enable to override BAUDRATE
 
 /**
@@ -2931,7 +2931,7 @@
 // RepRapDiscount FULL GRAPHIC Smart Controller
 // https://reprap.org/wiki/RepRapDiscount_Full_Graphic_Smart_Controller
 //
-//#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
+#define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
 
 //
 // K.3D Full Graphic Smart Controller
diff --git a/platformio.ini b/platformio.ini
index 76200cb..7f4f73c 100644
--- a/platformio.ini
+++ b/platformio.ini
@@ -13,7 +13,7 @@
 [platformio]
 src_dir      = Marlin
 boards_dir   = buildroot/share/PlatformIO/boards
-default_envs = mega2560
+default_envs = STM32F429ZG_btt
 include_dir  = Marlin
 extra_configs =
     Marlin/config.ini

Screenshot from the ESP3D interface after sending M115:
image

I've also tested this BTT ESP module on an Octopus Max EZ and it works as expected.

@dc740
Copy link
Contributor Author

dc740 commented Apr 24, 2024

wow, those are very interesting results. Then the module either has a newer chip version (remember I found this module in a drawer, I think I bought them when they started to become popular years ago), or the module already has CS to GND, so it always boot into Run mode. Nice. I can't wait for the module to better understand this. I want to see the differences in hardware. (then I will check the datasheets. It's been a busy week)

This means that the MR can be merged for now. I will still need to keep wifisupport enabled for the init code to run on my current setup, but that's expected.

I will leave the rest of the discussions on #26624

@thisiskeithb
Copy link
Member

MKS' ESP8266/ESP12S modules work the same way, so I think all of the ESP modules from these OEMs work without the changes in this PR.

The older generic ESP8266/ESP-01 modules (with 8-pins) also work fine on older motherboards when flashed with ESP3D and enabling a valid serial port in Marlin.

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 24, 2024

This means that the MR can be merged for now.

I hesitate to agree because this is for a one-off/custom module and not standard hardware. We already get repeated support questions related to standard ESP8266/ESP12S/ESP-01 modules from MKS & BTT and whether WIFISUPPORT should be enabled or not.

@dc740
Copy link
Contributor Author

dc740 commented Apr 24, 2024

the 8 pin ones don't expose GPIO15 at all, so it comes pulled down by default. I keep wondering why reprap does initialize this port, and why I was the first one to hit this case (or if others simply assumed it was not compatible).

My only theory so far is that newer chips don't have this requirement. But that's just a guess. I will dig into this over the weekend. I don't want to waste anyone's time.

@thisiskeithb thisiskeithb marked this pull request as draft April 24, 2024 08:10
@dc740
Copy link
Contributor Author

dc740 commented Apr 24, 2024

For reference, the documentation states that GPIO15 must be pulled down in order to boot:
https://docs.espressif.com/projects/esptool/en/latest/esp8266/advanced-topics/boot-mode-selection.html
image

This explains why RepRap is already doing it. It's the right thing to do. But I will just wait for the module to arrive to check if they are pulling GPIO15 down on hardware, or they let it floating and it just boots for no apparent reason and my quick and dirty protoboard just exposed the issue.

@thisiskeithb
Copy link
Member

thisiskeithb commented Apr 24, 2024

I will just wait for the module to arrive to check if they are pulling GPIO15 down on hardware, or they let it floating and it just boots for no apparent reason

Testing BTT's ESP8266/ESP-12S module here & I can access the ESP3D UI with with just 3.3V and GND connected (comms with motherboard obviously doesn't exist). GND and CS pin don't appear to have continuity on the module or motherboard, so maybe they're just floating. 🤷‍♂️

@dc740
Copy link
Contributor Author

dc740 commented Apr 29, 2024

Hi, I got the module on the email this morning. The newer chips already have a resistor to ground, this is why the BTT and MKS modules are booting fine.

Here is what I read on ESP-07S
image

And this is the schematic I use to flash the old one (plain ESP2866MOD) that doesn't come with GPIO15 pulled low by default:
esp8266-ESP-12E_Basic_flashing_connection

I will add a 10k resistor on my module so this MR can be closed. I still wonder if we should also pull GPIO15 down to follow the documentation instead of leaving it to the hardware implementation.

All in all, this has been sorted out and we now have all the information required. The ESP modules booting process is really something.

@thisiskeithb
Copy link
Member

I will add a 10k resistor on my module so this MR can be closed. I still wonder if we should also pull GPIO15 down to follow the documentation instead of leaving it to the hardware implementation.

I don't think it makes sense to add this change for a single motherboard, especially since the issue only exists with a non-standard ESP install. ESP modules from each board vendor are designed to work without the change.

The great debate about what "WIFISUPPORT" means will continue in #26624...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Peripherals Needs: Discussion Discussion is needed Needs: Work More work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants