search

MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.18k stars 19.21k forks source link

[FR] Proper EEPROM support on Fysetc S6 #18434

Closed bipsendk closed 3 years ago

bipsendk commented 4 years ago

Description

Add support for the onboard EEPROM for Fysetc S6

Feature Workflow

According to the documentation for Fysetc S6 there is an onboard 24LC16 EEPROM When compiling with PRINTCOUNTER enabled, an error is issued:

Marlin\src\HAL\STM32\../../inc/../HAL/STM32/inc/SanityCheck.h:44:4: warning: #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing." [-Wcpp]
   44 |   #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing."
      |    ^~~~~~~
Marlin\src\HAL\STM32\../../inc/../HAL/STM32/inc/SanityCheck.h:45:4: error: #error "Disable PRINTCOUNTER or choose another EEPROM emulation."

This is a bit wierd, as the controller has on-board EEPROM.

Also, a warning is issued on:
Marlin\src\HAL\STM32\../../inc/../HAL/STM32/inc/SanityCheck.h:44:4: warning: #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing." [-Wcpp]
   44 |   #warning "FLASH_EEPROM_EMULATION may cause long delays when writing and should not be used while printing."

But no FLASH_EEPROM_EMULATION definitions exist on configuration.h or configuration_adv.h

Additional Information

Schematics for Fysetc S6 can be found on https://github.com/FYSETC/FYSETC-S6

Tested with a pull of the current bugfix on June 26th, 2020

bipsendk commented 4 years ago

Found in pin_FYSETC_S6s.h -> seems to default to flash emulation, even though EEPROM exists.

Section should probably look like:

//
// EEPROM Emulation
//
#if NO_EEPROM_SELECTED
  //#define FLASH_EEPROM_EMULATION
  //#define SRAM_EEPROM_EMULATION
  #define I2C_EEPROM
#endif

#if ENABLED(FLASH_EEPROM_EMULATION)
  // Decrease delays and flash wear by spreading writes across the
  // 128 kB sector allocated for EEPROM emulation.
  #define FLASH_EEPROM_LEVELING
#elif ENABLED(I2C_EEPROM)
  #define MARLIN_EEPROM_SIZE 0x800               // 2KB
#endif
GMagician commented 4 years ago

OT really S6 has a eeprom chip? can't find in features

thisiskeithb commented 4 years ago

Which revision is your board? Maybe it was added at some point.

Edit: @GerogeFu - Do all versions of the S6 have a real EEPROM?

bipsendk commented 4 years ago

Not sure about versions prior to V1.2 - but it is shown in the schematics for V1.2. Unfortunately there is no silkscreen print on the PCB, but in the schematic it is shown on the first page as U8

In an Altium online viewer, it can also be loacted - screenshot of location of EEPROM is shown at https://ibb.co/rdmfwqL

GMagician commented 4 years ago

@thisiskeithb I don't have any, it was just a question because I thinking about buying one...

jimi0303 commented 4 years ago

Found in pin_FYSETC_S6s.h -> seems to default to flash emulation, even though EEPROM exists.

Section should probably look like:

//
// EEPROM Emulation
//
#if NO_EEPROM_SELECTED
  //#define FLASH_EEPROM_EMULATION
  //#define SRAM_EEPROM_EMULATION
  #define I2C_EEPROM
#endif

#if ENABLED(FLASH_EEPROM_EMULATION)
  // Decrease delays and flash wear by spreading writes across the
  // 128 kB sector allocated for EEPROM emulation.
  #define FLASH_EEPROM_LEVELING
#elif ENABLED(I2C_EEPROM)
  #define MARLIN_EEPROM_SIZE 0x800               // 2KB
#endif

Hello! I'm trying to get the EEPROM working on my Fysetc S6. I've had this board for three months. Here is my first problem/solution for EEPROM:

I'd like to state that I have no experience in programming so bear with me please. Until my EEPROM module arrives (which now could take 90 days) I'd still like to use my printer with this board. I bought it used without an SD card, so obviously it did not work, the seller disappeared so it took me a good while to figure out the problem.

I managed to get it printing by saving the EEPROM to the SD card on a TFT35 v3.0 Here is the problem: obviously if I pull the SD card while the printer is on to upload a .gcode from my computer then put it back, the EEPROM doesn't work (can't explain why, guess it can't read it again or something..). So what I did until now was power cycle the printer so the settings are restored from the SD card. But now it does not seem to work reliably. I use auto bed leveling (I get that that data is stored in the EEPROM) and when I start a print I can see it is correcting the skew of the bed on the Z axis, but after some of the first lines the axis stops completely, the XY axes are still working and the print is going. So the print just goes on trying to mush layers in each other because the Z axis just stops completely, the stepper motor turns off, it doesn't raise Z even on layer changes, you can turn the axis by hand while the print is still going. Now I have to power cycle the printer 2-3 times to get it working. It seems like if I do anything in the printer menu before starting the print, then this will happen. Or not. I can't really explain it, it's pretty random.

This was 1,5 months ago. Now my I2C chip has arrived, but I can't get it working. I commented out flash EEPROM emulation and enabled I2C EEPROM in pins.h, yet it does not save there. Without the SD card in there is no boot logo, only a black screen and pressing the knob repeatedly brings forth the menu after like a minute. I don't know what to make of this or how to make it work. When I insert the SD it boots up normally. But I checked and there is no longer an EEPROM file saved on the SD card, so I don't really understand what is going on. Could you help me? I'll try your bit of code in pins.h that you have written here.

bipsendk commented 4 years ago

Maybe these lines need to be added as well in pins_FYSETC_S6.h

define IIC_EEPROM_SDA PB9

define IIC_EEPROM_SCL PB8

At least - that is what I can see from the schematic.

It could also be:

define I2C_SCL_PIN PB8

define I2C_SDA_PIN PB9

And maybe even a

define IIC_BL24CXX_EEPROM

needs to be there ?

I am not quite sure which of them to use - one of the developers will have to give some input.

I have a Fysetc S6 board, it just arrived a couple of days ago... But I am still missing the stepper drrivers, so it has not been hooked up to a printer yet. I have just managed to get the marlin firmware to compile without issues (for my CoreXY setup), next step will be to try to load the firmware to the board - to verify whether the EEPROM bit actually works as expected.

jimi0303 commented 4 years ago

Thank you for the fast reply! I copied your bit of code over the original EEPROM Emulation settings, with only the I2C line enabled, in hope that it will work properly. What happened was it was saving again only on the SD card, so I couldn't get either the board itself or the I2C chip to retain any information between power cycles. To be honest I think the I2C chip never worked, it didn't make a difference if it was plugged in or not. So now I'm back one step again. Getting pretty clueless here, I hoped that hooking up an EEPROM module and uncommenting I2C EEPROM would solve my original bug from here: https://github.com/MarlinFirmware/Marlin/issues/16060

bipsendk commented 4 years ago

From trying to understand the code, it looks like what is needed is something like:

//
// EEPROM Emulation
//
#if NO_EEPROM_SELECTED
  //#define FLASH_EEPROM_EMULATION
  //#define SRAM_EEPROM_EMULATION
  #define I2C_EEPROM
#endif

#if ENABLED(FLASH_EEPROM_EMULATION)
  // Decrease delays and flash wear by spreading writes across the
  // 128 kB sector allocated for EEPROM emulation.
  #define FLASH_EEPROM_LEVELING
#elif ENABLED(I2C_EEPROM)
  #define IIC_BL24CXX_EEPROM
  #define IIC_EEPROM_SDA PB9
  #define IIC_EEPROM_SCL PB8
  #define MARLIN_EEPROM_SIZE 0x800               // 2KB
#endif

Not sure if this is the exact solution....

jimi0303 commented 4 years ago

Thank you, I will try that tomorrow and respond back with the results. Though I don't understand the first part with "#if NO_EEPROM_SELECTED". Where would I select EEPROM in the first place?

bipsendk commented 4 years ago

No default section/display exists in configuration.h/configuration_adv.h - I was a bit puzzled about this as well..

Just found out, that I cannot compile if I enable IIC_BL24CXX_EEPROM ... So I guess we will have to wait for input from a developer..

sjasonsmith commented 4 years ago

There is no on-board EEPROM on my S6 V1.2, nor on the pictures FYSETC publishes.

This is where it would go, if they bothered to populate it: image

sjasonsmith commented 4 years ago

Unless somebody can provide a picture showing that part populated on an S6 from FYSETC, this feature request isn't really valid and should be closed.

It is hard to know whether the EEPROM was simply not populated to reduce cost, or if there is a problem on the board that prevented it from working.

bipsendk commented 4 years ago

I will post a picture later today of my board - as I am not at home at the moment.

jimi0303 commented 4 years ago

@sjasonsmith You also closed the other issue that I referenced here, saying it was "probably" fixed, yet not answering my question regarding to what and where should I update to get the fix, so my machine is still as useless as they come. Could you be bothered to answer it now?

"Thanks for the input, honestly this whole firmware thing was more than a month ago and I'm not even sure what I should update anymore. I remember that I couldn't compile the provided firmwares so I whipped up a vanilla Marlin from scratch and made the settings so it would fit my Sapphire Pro and my preferences. Of course I dated everything, so I have those files. Could you shed some light on what needs to be updated so this issue is fixed?"

sjasonsmith commented 4 years ago

@jimi0303, sorry I missed the question at the end of your comment on the other issue. I probably read the part about not having touched it in a month and glanced over the rest. I read a lot of comments and miss things from time to time.

FLASH EEPROM Emulation just works without doing anything special. Just uncomment EEPROM_SETTINGS in your Configuration.h and you are good to go. I'm not positive whether it works fine in 2.0.5.3, or if you need to use the bugfix-2.0.x branch.

I said "Probably" when I closed the issue because I couldn't remember whether I had fixed the STM32F4 flash issues on the S6 or an SKR Pro. I knew other people were successfully using the board with flash-based EEPROM, and that was an old issue from literally the second person I saw trying to put Marlin on that board.

gerleimarci commented 4 years ago

IMG_20200629_144254

jimi0303 commented 4 years ago

@sjasonsmith Thank you for the answer! I think I'm getting to the end of this problem. So, I confirmed that I have Marlin 2.0.5.3 on my printer, I managed to get the flash-based EEPROM working (couldn't get the I2C), but: when no SD card is inserted before powering on the printer, the LCD hangs on a full black screen - after holding the knob for a random amount of time - sometimes 15s, sometimes a minute, the menu comes of as if nothing happened. If I power on the printer with the SD card inserted, it boots up normally with the logo and everything. I can confirm that it is not using the SD card as EEPROM storage, because no EEPROM file is saved on it and the printer retains information without the card in.

So I'd like to solve this last problem. Either with this older marlin or the bugfix version. I've spent hours on configuring Marlin to my printer and needs (no programming skills), so I'd really like to just copy over my settings to the newer version of Marlin, not start from scratch again. Is that possible? Or is it possible to fix this black screen issue with the older software? Thank you in advance.

bipsendk commented 4 years ago

@gerleimarci - Thanks - then I won't have to upload a picture as well :-)

bipsendk commented 4 years ago

image

bipsendk commented 4 years ago

At least - if it just was easier to enable the onboard EEPROM (if present) just by adding

define I2C_EEPROM

then things would be much simpler to utilize the onboard memory eeprom :-)

sjasonsmith commented 4 years ago

Thanks for the pictures. Are the boards that shipped with EEPROMs still V1.2? It is printed in the lower-right corner of my board.

Using the EEPROM as the default causes a bit of a problem if they have shipped boards both ways, without a good way to identify them.

bipsendk commented 4 years ago

Silkscreen still says v1.2 - unfortunately. But if everything else is prepared in the pins.h file regarding the eeprom size, the SDA/SCL pins and so on - so that it only is a question of defining I2C_EEPROM - then it will just be a matter of identifying whether the component is mounted or not. If the SOT23 component is mounted, the user can enable I2C_EEPROM - otherwise flash emulation or similar will have to be used.

sjasonsmith commented 4 years ago

@bipsendk I agree with you that is the best option. It might be best to fill in both options, and use #error statements to prevent the board from building until one of the methods is selected.

Unfortunately I damaged my board yesterday doing some other testing, and won't be able to contribute directly to this. I have parts on order to hopefully fix it and add my missing EEPROM, but it will be a few days before they arrive.

sjasonsmith commented 4 years ago

@jimi0303 I'm not sure what is impacting that startup issue. I suggest you start with a clean minimal config, to reduce the chance that your other configurations are contributing to that delay. If it works, then you can then add your changes in slowly to see if the problem appears with one of them.

It would be best to separate that issue from this feature request, since they do not seem related. I recommend seeking help from other users to see if they have had similar problems on Discord or one of the other user forums. If there then seems to be a Marlin bug related to this you could report a new issue.

GerogeFu commented 4 years ago

Which revision is your board? Maybe it was added at some point.

Edit: @GerogeFu - Do all versions of the S6 have a real EEPROM?

First batch of the S6 don't have real EEPROM , so the default EEPROM option is FLASH_EEPROM_EMULATION.

GerogeFu commented 4 years ago

@sjasonsmith Thank you for the answer! I think I'm getting to the end of this problem. So, I confirmed that I have Marlin 2.0.5.3 on my printer, I managed to get the flash-based EEPROM working (couldn't get the I2C), but: when no SD card is inserted before powering on the printer, the LCD hangs on a full black screen - after holding the knob for a random amount of time - sometimes 15s, sometimes a minute, the menu comes of as if nothing happened. If I power on the printer with the SD card inserted, it boots up normally with the logo and everything. I can confirm that it is not using the SD card as EEPROM storage, because no EEPROM file is saved on it and the printer retains information without the card in.

So I'd like to solve this last problem. Either with this older marlin or the bugfix version. I've spent hours on configuring Marlin to my printer and needs (no programming skills), so I'd really like to just copy over my settings to the newer version of Marlin, not start from scratch again. Is that possible? Or is it possible to fix this black screen issue with the older software? Thank you in advance.

The old bootloader may cause this issue , you can try to update the bootloader here : https://github.com/FYSETC/Bootloader-S6

bipsendk commented 4 years ago

@sjasonsmith - do you have any input on whether #define IIC_BL24CXX_EEPROM should be used or not ? I had it enabled in a compile attempt, but get this error:

Marlin\src\lcd/dwin/eeprom_BL24CXX.h:29:10: fatal error: libmaple/gpio.h: No such file or directory

But if the define shouldn't be used, there is no need to dig further into it...

sjasonsmith commented 4 years ago

@bipsendk, I don't know, but it certainly should not be including anything from a libmaple folder for a FYSETC S6. The STM32 HAL this board uses does not rely on Maple.

thinkyhead commented 4 years ago

IIC_BL24CXX_EEPROM

Only Ender 3 V2 uses this, and only to store its some data within the lcd/dwin code. At some point we'll make a proper HAL PersistentStore class out of it.

bipsendk commented 4 years ago

FYI: Got my 2nd S6 board today, and it also has an EEPROM mounted on the PCB.

bipsendk commented 4 years ago

Not sure whether to open a new ticket. I just tried with the bugfix branch from today (20200727), and with changing pins_FYSETC_S6.h to have:

//
// EEPROM Emulation
//
#if NO_EEPROM_SELECTED
  //#define FLASH_EEPROM_EMULATION
  //#define SRAM_EEPROM_EMULATION
  #define I2C_EEPROM
#endif

#if ENABLED(FLASH_EEPROM_EMULATION)
  // Decrease delays and flash wear by spreading writes across the
  // 128 kB sector allocated for EEPROM emulation.
  #define FLASH_EEPROM_LEVELING
#elif ENABLED(I2C_EEPROM)
  #define IIC_EEPROM_SDA PB9
  #define IIC_EEPROM_SCL PB8
  #define MARLIN_EEPROM_SIZE 0x800               // 2KB
#endif

Compile of the bugfix branch from 20200726 worked OK.

bugfix-2.0.x from 20200727 gives me (with same modification to pins file and same configuration.h/configuration_adv.h files):

Compiling .pio\build\FYSETC_S6\src\src\HAL\STM32\eeprom_impl.cpp.o
Marlin\src\HAL\STM32\eeprom_impl.cpp: In static member function 'static bool PersistentStore::write_data(int&, const uint8_t*, size_t, uint16_t*)':
Marlin\src\HAL\STM32\eeprom_impl.cpp:48:33: error: invalid conversion from 'uint8_t*' {aka 'unsigned char*'} to 'uint32_t' {aka 'long unsigned int'} [-fpermissive]
   48 |       if (v != eeprom_read_byte(p)) {
      |                                 ^
      |                                 |
      |                                 uint8_t* {aka unsigned char*}
In file included from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/board.h:18,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/wiring.h:41,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/Arduino.h:32,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/../shared/Marduino.h:36,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/HAL.h:28,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/HAL.h:26,
                 from Marlin\src\HAL\STM32\../../inc/MarlinConfig.h:30,
                 from Marlin\src\HAL\STM32\eeprom_impl.cpp:25:
C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino\stm32/stm32_eeprom.h:63:41: note:   initializing argument 1 of 'uint8_t eeprom_read_byte(uint32_t)'
   63 | uint8_t eeprom_read_byte(const uint32_t pos);
      |                          ~~~~~~~~~~~~~~~^~~
Marlin\src\HAL\STM32\eeprom_impl.cpp:49:27: error: invalid conversion from 'uint8_t*' {aka 'unsigned char*'} to 'uint32_t' {aka 'long unsigned int'} [-fpermissive]
   49 |         eeprom_write_byte(p, v);
      |                           ^
      |                           |
      |                           uint8_t* {aka unsigned char*}
In file included from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/board.h:18,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/wiring.h:41,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/Arduino.h:32,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/../shared/Marduino.h:36,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/HAL.h:28,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/HAL.h:26,
                 from Marlin\src\HAL\STM32\../../inc/MarlinConfig.h:30,
                 from Marlin\src\HAL\STM32\eeprom_impl.cpp:25:
C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino\stm32/stm32_eeprom.h:64:33: note:   initializing argument 1 of 'void eeprom_write_byte(uint32_t, uint8_t)'
   64 | void eeprom_write_byte(uint32_t pos, uint8_t value);
      |                        ~~~~~~~~~^~~
Marlin\src\HAL\STM32\eeprom_impl.cpp:50:30: error: invalid conversion from 'uint8_t*' {aka 'unsigned char*'} to 'uint32_t' {aka 'long unsigned int'} [-fpermissive]
   50 |         if (eeprom_read_byte(p) != v) {
      |                              ^
      |                              |
      |                              uint8_t* {aka unsigned char*}
In file included from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/board.h:18,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/wiring.h:41,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/Arduino.h:32,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/../shared/Marduino.h:36,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/HAL.h:28,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/HAL.h:26,
                 from Marlin\src\HAL\STM32\../../inc/MarlinConfig.h:30,
                 from Marlin\src\HAL\STM32\eeprom_impl.cpp:25:
C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino\stm32/stm32_eeprom.h:63:41: note:   initializing argument 1 of 'uint8_t eeprom_read_byte(uint32_t)'
   63 | uint8_t eeprom_read_byte(const uint32_t pos);
      |                          ~~~~~~~~~~~~~~~^~~
Marlin\src\HAL\STM32\eeprom_impl.cpp: In static member function 'static bool PersistentStore::read_data(int&, uint8_t*, size_t, uint16_t*, bool)':
Marlin\src\HAL\STM32\eeprom_impl.cpp:72:26: error: invalid conversion from 'uint8_t*' {aka 'unsigned char*'} to 'uint32_t' {aka 'long unsigned int'} [-fpermissive]
   72 |         eeprom_read_byte((uint8_t*)pos)
      |                          ^~~~~~~~~~~~~
      |                          |
      |                          uint8_t* {aka unsigned char*}
In file included from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/board.h:18,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/wiring.h:41,
                 from C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino/Arduino.h:32,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/../shared/Marduino.h:36,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/./STM32/HAL.h:28,
                 from Marlin\src\HAL\STM32\../../inc/../HAL/HAL.h:26,
                 from Marlin\src\HAL\STM32\../../inc/MarlinConfig.h:30,
                 from Marlin\src\HAL\STM32\eeprom_impl.cpp:25:
C:\users\brian.ipsen\.platformio\packages\framework-arduinoststm32@3.10700.191028\cores\arduino\stm32/stm32_eeprom.h:63:41: note:   initializing argument 1 of 'uint8_t eeprom_read_byte(uint32_t)'
   63 | uint8_t eeprom_read_byte(const uint32_t pos);
      |                          ~~~~~~~~~~~~~~~^~~
*** [.pio\build\FYSETC_S6\src\src\HAL\STM32\eeprom_impl.cpp.o] Error 1
sjasonsmith commented 3 years ago

This should work with Marlin today. It is the default for S6 V2.0 boards. V1.2 boards do not use this as the default since not all shipped boards have EEPROM's populated.

If you wish to use the I2C EEPROM on your V1.2 board, simple add the following line somewhere in your Configuration.h file: #define I2C_EEPROM

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.