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

Initial BTT SKR Mini E3 V2 support #18088

Merged

Conversation

thisiskeithb
Copy link
Member

@thisiskeithb thisiskeithb commented May 24, 2020

Description

Initial SKR Mini E3 V2 support. I don't have one on hand yet, so I marked this PR as draft so it can be tested.

BigTreeTech changed two pin numbers in the EXP1 connector, but I tried to keep this PR "DRY" since pins_BTT_SKR_MINI_E3.h still contains most of the correct pins. If there's a better way to handle this, like moving the EXP pins to their respective SKR Mini E3 pins files, that would be fine.

To-do:

  • Convert old E2END EEPROM method to MARLIN_EEPROM_SIZE
  • Buy a V2 board off Amazon since shipping from China is taking 2+ months
  • Figure out why defining I2C_EEPROM in pins file still returns NO_EEPROM_SELECTED/FLASH_EEPROM_EMULATION. Likely a file include order issue.
  • Figure out why using onboard I2C_EEPROM causes lockup on boot with latest Marlin/after commit 5f7a759. Using emulated EEPROM works fine.

Benefits

SKR Mini E3 V2 support.

Related Issues

V2 config is marked as draft & ready for testing/review: MarlinFirmware/Configurations#111

@Lord-Quake
Copy link
Contributor

@thisiskeithb Nice. I should be getting my board on Monday and hope to have my Ender 3 Pro up and running again for tests.

@thisiskeithb
Copy link
Member Author

thisiskeithb commented May 24, 2020

I should be getting my board on Monday and hope to have my Ender 3 Pro up and running again for tests.

That'd be great. I have one coming direct from BigTreeTech, but it will likely take 1-2mos at the rate international shipping is going right now.

Edit: Config is located in MarlinFirmware/Configurations#111. It's obviously not too different than the other two Mini E3 configs, but it should be "ready to go".

@thisiskeithb thisiskeithb changed the title Initial SKR Mini E3 V2 support Initial BTT SKR Mini E3 V2 support May 24, 2020
@Lord-Quake
Copy link
Contributor

Lord-Quake commented May 25, 2020

@thisiskeithb I have my new board installed. I can now test with the Ender 3. Let me know what I can do to get the 2.0 board up and running.

Edit: I was able to successfully compile your branch https://github.com/thisiskeithb/Marlin/tree/pr/add_skr_mini_e3_v2 with a few modifications. Tomorrow I will convert the configurations exactly to my needs. Then we will see what happens....

@thisiskeithb
Copy link
Member Author

Edit: I was able to successfully compile your branch https://github.com/thisiskeithb/Marlin/tree/pr/add_skr_mini_e3_v2 with a few modifications.

Which modifications were required?

@Lord-Quake
Copy link
Contributor

Lord-Quake commented May 26, 2020

It would not compile until I changed this in platform.ini

;Adafruit_MAX31865=https://github.com/adafruit/Adafruit_MAX31865/archive/master.zip
Adafruit_MAX31865=https://github.com/adafruit/Adafruit_MAX31865/archive/1.1.0.zip

Edit: I see now it was taken care with commit thisiskeithb@f402ab7

Edit1: Now getting the following error with the latest commit:

In file included from Marlin\src\HAL\STM32F1\../../inc/../pins/pins.h:516:0,
                 from Marlin\src\HAL\STM32F1\../../inc/MarlinConfig.h:32,
                 from Marlin\src\HAL\STM32F1\HAL.cpp:30:
Marlin\src\HAL\STM32F1\../../inc/../pins/stm32f1/pins_BTT_SKR_MINI_E3_V2_0.h:34:0: warning: "MARLIN_EEPROM_SIZE" redefined
 #define MARLIN_EEPROM_SIZE 0x1000                 // 4KB

In file included from Marlin\src\HAL\STM32F1\../../inc/../pins/stm32f1/pins_BTT_SKR_MINI_E3_V2_0.h:25:0,
                 from Marlin\src\HAL\STM32F1\../../inc/../pins/pins.h:516,
                 from Marlin\src\HAL\STM32F1\../../inc/MarlinConfig.h:32,
                 from Marlin\src\HAL\STM32F1\HAL.cpp:30:
Marlin\src\HAL\STM32F1\../../inc/../pins/stm32f1/pins_BTT_SKR_MINI_E3.h:38:0: note: this is the location of the previous definition
   #define MARLIN_EEPROM_SIZE    EEPROM_PAGE_SIZE  // 2KB

In file included from Marlin\src\HAL\STM32F1\../../inc/MarlinConfig.h:40:0,
                 from Marlin\src\HAL\STM32F1\HAL.cpp:30:
Marlin\src\HAL\STM32F1\../../inc/SanityCheck.h:2061:6: error: #error "Please select only one method of EEPROM Persistent Storage."
     #error "Please select only one method of EEPROM Persistent Storage."
      ^~~~~

@thisiskeithb
Copy link
Member Author

thisiskeithb commented May 26, 2020

@thinkyhead: Do we have to #undef MARLIN_EEPROM_SIZE in the pins files?

Edit: I don't know why this block is being called when defining EEPROM_SETTINGS in Configuration.h:
https://github.com/thisiskeithb/Marlin/blob/3d433d0b95c23a7eb6dad33aeafaef87a944363f/Marlin/src/pins/stm32f1/pins_BTT_SKR_MINI_E3.h#L34-L39

#if EITHER(NO_EEPROM_SELECTED, FLASH_EEPROM_EMULATION)
  #define FLASH_EEPROM_EMULATION
  #define EEPROM_PAGE_SIZE     (0x800U) // 2KB
  #define EEPROM_START_ADDRESS (0x8000000UL + (STM32_FLASH_SIZE) * 1024UL - (EEPROM_PAGE_SIZE) * 2UL)
  #define MARLIN_EEPROM_SIZE    EEPROM_PAGE_SIZE  // 2KB
#endif

The V2 has a dedicated I2C EEPROM:
https://github.com/thisiskeithb/Marlin/blob/3d433d0b95c23a7eb6dad33aeafaef87a944363f/Marlin/src/pins/stm32f1/pins_BTT_SKR_MINI_E3_V2_0.h#L32-L34

// Onboard I2C EEPROM
#define I2C_EEPROM
#define MARLIN_EEPROM_SIZE 0x1000                 // 4KB

Moving the I2C EEPROM code before the pins_BTT_SKR_MINI_E3.h include doesn't help either.

@Lord-Quake
Copy link
Contributor

Lord-Quake commented May 26, 2020

Compilation is successful when the lines are removed

//  #define FLASH_EEPROM_EMULATION
//  #define EEPROM_PAGE_SIZE     (0x800U) // 2KB
//  #define EEPROM_START_ADDRESS (0x8000000UL + (STM32_FLASH_SIZE) * 1024UL - (EEPROM_PAGE_SIZE) * 2UL)
//  #define MARLIN_EEPROM_SIZE    EEPROM_PAGE_SIZE  // 2KB

Like you stated, now to figure out why it is being called in the fist place.

Edit: The printer freezes while the #define SHOW_BOOTSCREEN is showing.

@thisiskeithb
Copy link
Member Author

Edit: The printer freezes while the #define SHOW_BOOTSCREEN is showing.

Does it work if you back out the MARLIN _EEPROM_SIZE commit and use this instead:

// Onboard I2C EEPROM
 #define I2C_EEPROM
 #undef E2END
 #define E2END       (0xFFF) // 4KB

@Lord-Quake
Copy link
Contributor

I have to go to work..... Will try later in the day.

@thisiskeithb
Copy link
Member Author

thisiskeithb commented May 26, 2020

There's probably something up with the include order causing this to never register that both EEPROM_SETTINGS is set in the config and I2C_EEPROM is set in the pins, so it will always define NO_EEPROM_SELECTED.

// Flag if an EEPROM type is pre-selected
#if ENABLED(EEPROM_SETTINGS) && NONE(I2C_EEPROM, SPI_EEPROM, QSPI_EEPROM, FLASH_EEPROM_EMULATION, SRAM_EEPROM_EMULATION, SDCARD_EEPROM_EMULATION)
#define NO_EEPROM_SELECTED 1
#endif

Edit: I could always #undef NO_EEPROM_SELECTED in the pins file, but that seems hacky.

@Lord-Quake
Copy link
Contributor

Lord-Quake commented May 26, 2020

I grabbed commit f7346b6 and used board version 1.2. Then I made all the necessary pin changes and added the external eeprom. No compiler error at all but no matter what I try the SHOW_BOOTSCREEN freezes.

A bit frustrating..... (also because I'm no pro with Marlin coding)

@sjasonsmith
Copy link
Contributor

External EEPROM? Doesn’t that board already have an EEPROM built in?

@thisiskeithb
Copy link
Member Author

External EEPROM? Doesn’t that board already have an EEPROM built in?

Yep. The SKR Mini E3 V2 has an onboard EEPROM like the GTR.

@Lord-Quake
Copy link
Contributor

They added a AT24C23 to the board. Was my wording as "external" incorrect?

@sjasonsmith
Copy link
Contributor

They added a AT24C23 to the board. Was my wording as "external" incorrect?

I assumed by external you meant you wired an additional EEPROM to the board, as many do for boards without EEPROMs.

@JeremyKennedy
Copy link

I tried building Marlin using the changes from this PR plus your configuration PR, but I got the EEPROM error as above which prevented it from building successfully. I tried the workaround of adding #undef NO_EEPROM_SELECTED which allowed it to build, however when loading it to my printer I get stuck on the bootscreen as also mentioned above.

I pulled everything from the official BTT repo, and that was able to build and load onto my printer without issue.

It is hard to say exactly what the differences are between this PR and the BTT repo that cause the issue. I found that the BTT repo is based off commit 27c281e which was March 22, so a fair bit of things have changed since then, including EEPROM and SDCARD handling/configs. Looking at the diff between commit 27c281e and the BTT repo shows exactly what BTT changed to get Marlin working, but I can't figure out how to apply that to more recent versions.

As of now I'm at a loss, I've tried dozens of different firmware builds to no avail. For now I'm going to stick with my branch off of commit 27c281e with BTT's fixes (pins/boards) and your configuration. The only part I had to pull from BTT's configuration was the slave addresses - without that, my y-axis movement was essentially compressed to a few centimeters - my Benchy was under 1cm wide (x-axis was fine, though). Once I applied that change I was good to go.

Let me know if I can help or test.

@Lord-Quake
Copy link
Contributor

@JeremyKennedy I did it very similar as you however I haven't gotten to the point where I can print. Right now I'm stuck where I can't home the Z-axis. I'm using a 3D-Touch with my setup. Could you please post your configs (even though you might not a level sensor) so I can take a look at your settings.

@thisiskeithb
Copy link
Member Author

thisiskeithb commented May 27, 2020

It is hard to say exactly what the differences are between this PR and the BTT repo that cause the issue. I found that the BTT repo is based off commit 27c281e which was March 22, so a fair bit of things have changed since then, including EEPROM and SDCARD handling/configs.

I tried to make the V2 pins fit recent Marlin changes & not end up repeating the same pin definitions, but I'm not sure why it's not working as expected.

The only part I had to pull from BTT's configuration was the slave addresses

Thanks for the note. I've updated the config to include those changes.

Edit: I also ordered a V2 board off Amazon today so I can debug this locally.

@Lord-Quake
Copy link
Contributor

Well, never trust a manufacturer and the configs they provide. Got my 3DTouch sorted now by having to edit the pins_BTT_SKR_MINI_E3.h file. I thought I had a DOA board. I can now continue....

@thisiskeithb
Copy link
Member Author

Got my 3DTouch sorted now by having to edit the pins_BTT_SKR_MINI_E3.h file.

Which edits did you have to make?

@Lord-Quake
Copy link
Contributor

For some reason only Z_STOP_PIN is recognized.

#define Z_STOP_PIN PC14 // was PC2
and
//#define Z_MIN_PROBE_PIN PC14

@thisiskeithb
Copy link
Member Author

Did you enable USE_PROBE_FOR_Z_HOMING? That should force Marlin to use the designated probe pin when you install a probe.

@Lord-Quake
Copy link
Contributor

In what config as I can't find that declaration?

@thisiskeithb
Copy link
Member Author

In what config as I can't find that declaration?

Search Configuration.h for USE_PROBE_FOR_Z_HOMING. Here's the direct link in the SKR Mini E3 V2 config I submitted.

@Lord-Quake
Copy link
Contributor

Thanks Keith but I'm using https://github.com/bigtreetech/BIGTREETECH-SKR-mini-E3/tree/master/firmware/V2.0/Marlin-2.0.x-SKR-mini-E3-V2.0 for now so that I have a working printer and know the new board is working.
Right now I haven't found a solution with the freezing of the printer with your configs.

@randellhodges
Copy link
Contributor

@JeremyKennedy See my edit with the proper place to add eeprom_init.

If it works, thank @pkilar for finding it. If it doesn't work, blame someone other than me :)

@JeremyKennedy
Copy link

@JeremyKennedy See my edit with the proper place to add eeprom_init.

If it works, thank @pkilar for finding it. If it doesn't work, blame someone other than me :)

Ha, I deleted my comment after seeing yours and realizing it's probably more complicated than I expected! But after seeing this latest one, maybe I can try playing around with it.

@thinkyhead
Copy link
Member

eeprom_write_byte and eeprom_read_byte functions need eeprom_init

Code that wants to access the EEPROM should be calling PersistentStore::access_start which calls eeprom_init when appropriate. However access_start in STM32F1/eeprom_wired.cpp doesn't call eeprom_init. So that's where the patch is needed.

@pkilar
Copy link

pkilar commented May 31, 2020

Removed eeprom_init() calls from from eeprom_if_i2c.cpp and made this change:

--- a/Marlin/src/HAL/STM32F1/eeprom_wired.cpp
+++ b/Marlin/src/HAL/STM32F1/eeprom_wired.cpp
@@ -47,6 +47,8 @@ bool PersistentStore::access_start() {
       SET_OUTPUT(SPI_EEPROM1_CS);
     #endif
     spiInit(0);
+  #elif ENABLED(I2C_EEPROM)
+    eeprom_init();
   #endif
   return true;
 }

my Ender starts properly now. I really want to start using the new board ;)

@thinkyhead
Copy link
Member

thinkyhead commented May 31, 2020

Removed eeprom_init() calls from from eeprom_if_i2c.cpp and made this change…

You must be looking at older code. There are no eeprom_init calls in eeprom_if_i2c.cpp at this time.

Ah, removed the ones you had added… Gotcha.

@thinkyhead thinkyhead marked this pull request as ready for review May 31, 2020 03:56
@thinkyhead
Copy link
Member

thinkyhead commented May 31, 2020

Figure out why defining I2C_EEPROM in pins file still returns NO_EEPROM_SELECTED / FLASH_EEPROM_EMULATION

  • First, NO_EEPROM_SELECTED is defined specifically for the benefit of pins files, to inform them they may provide a default. It is not referenced anywhere outside of pins_MY_BOARD.h files. It could be named PINDEFS_CAN_SET_EEPROM.

  • FLASH_EEPROM_EMULATION is not set by anything in the paths for these BTT boards, unless the tests or configs are setting it.

@thisiskeithb
Copy link
Member Author

  • First, NO_EEPROM_SELECTED is defined specifically for the benefit of pins files, to inform them they may provide a default. It is not referenced anywhere outside of pins_MY_BOARD.h files. It could be named PINDEFS_CAN_SET_EEPROM.
  • FLASH_EEPROM_EMULATION is not set by anything in the paths for these BTT boards, unless the tests or configs are setting it.

This code is setting NO_EEPROM_SELECTED (thus causing FLASH_EEPROM_EMULATION to be true) even though I've defined I2C_EEPROM in the V2 pins file, so that's why I'm trying to track down:

// Flag if an EEPROM type is pre-selected
#if ENABLED(EEPROM_SETTINGS) && NONE(I2C_EEPROM, SPI_EEPROM, QSPI_EEPROM, FLASH_EEPROM_EMULATION, SRAM_EEPROM_EMULATION, SDCARD_EEPROM_EMULATION)
#define NO_EEPROM_SELECTED 1
#endif

@thinkyhead
Copy link
Member

thinkyhead commented May 31, 2020

thus causing FLASH_EEPROM_EMULATION to be true

This would thus be occurring in a pins file. Some pins files include other pins files. So, if one of the pins file includes another pins file, and both pins files want to provide an EEPROM type, then both pins files need to first check NO_EEPROM_SELECTED and if the pins file ends up setting an EEPROM type then it should undef NO_EEPROM_SELECTED to let the next pins file in the chain know that it's all set.

@thinkyhead thinkyhead merged commit 3430d45 into MarlinFirmware:bugfix-2.0.x May 31, 2020
@thisiskeithb
Copy link
Member Author

Alright. I’m still waiting for my board so if someone can confirm that onboard EEPROM doesn’t cause lockups after recent patches, that’d be helpful.

@Lord-Quake
Copy link
Contributor

Lord-Quake commented May 31, 2020

Good news everyone, compilation was error free and the printer now boots up as expected 👍

Out of curiosity; Is there some way to find out if the "new" eeprom is being addressed?

@thisiskeithb thisiskeithb deleted the pr/add_skr_mini_e3_v2 branch May 31, 2020 05:18
@JeremyKennedy
Copy link

JeremyKennedy commented May 31, 2020

I can also confirm that Marlin builds on the latest 2.0 bugfix commit without any code changes! Thanks everyone all for the work on this.

@notLiria
Copy link

notLiria commented Jun 1, 2020

Can confirm that this will work (As in not lock up).

Delta configuration seems to be broken though.

@thisiskeithb
Copy link
Member Author

Delta configuration seems to be broken though.

What delta configuration? This PR was for the SKR Mini E3 V2.0 which is for an Ender-3.

@notLiria
Copy link

notLiria commented Jun 1, 2020

Micromake D1; I changed the settings in configuration.h and configuration_adv.h to delta settings according to my previous configuration.h and it appears that one of the steppers is moving significantly more than the others. Also compared my config to working Delta configs for SKR mini 1.2 and 1.3 from other people and found no major differences. It is possibly an error on my end but I am still investigating; if anybody has a Delta that they can try to plug the 2.0 into that would be great

Attached is a video documenting this behaviour (Commands sent are: G91, G1 X0 Y0 Z10, G1 X0 Y0 Z-10). The stepper that is moving significantly more is stepper output X in the configuration, and I have verified that this behaviour persists and is tied to the stepper output rather than the physical stepper itself.

https://streamable.com/yiy3uz

See here for my configuration files.
https://gist.github.com/notLiria/6b08e97d4065faf825719e3ee878d52f

@thisiskeithb
Copy link
Member Author

I'd recommend joining the Marlin Discord or posting in one of the other resources listed here.

@marcjwebb
Copy link

I have found another issue over the last couple of days with the new V2 board. I have tried this over multiple branches and no change at all.
it would appear that the eprom is not recalling saved parameters. more specifically, the z probe offset.

with the z probe offset set to -1.89 ( which should be bang on based from baby stepping value ) I still need to baby step no matter what. I have re-flashed firmware now over 20 times, and used M502 and going between branches.

I even changed my z probe offset to -1.78 ( should now be double the amount away ) but no, still baby stepping is required to +0.11

its like it is completely ignoring the eeprom settings ( strangely just the z as x and y dont seem to be a problem )

is there a way I can completely wipe the board and eeprom ?

or is there a way I can burn a boot loader and do it the old fashioned way? this has been over a week now and even when doing initialise eprom, restore defaults nothing works. about to send it back to china

@thisiskeithb
Copy link
Member Author

it would appear that the eprom is not recalling saved parameters. more specifically, the z probe offset.

Saving/recalling to EEPROM works fine on my V2 using the latest bugfix-2.0.x from this branch & V2 config, so you likely have a hardware issue.

@Lord-Quake
Copy link
Contributor

Lord-Quake commented Jun 3, 2020

Been using mine for over a week, constantly doing updates and have not encountered your issue.

You could deactivate the "external" eeprom on the board and use the chips CPU eeprom and check what happens.

jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.