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

Update ststm32 to 8.0 for HAL/STM32 #18496

Merged
merged 10 commits into from
Sep 8, 2020

Conversation

sjasonsmith
Copy link
Contributor

@sjasonsmith sjasonsmith commented Jul 2, 2020

Description

Update to the latest version of the stm32duino framework, used by HAL/STM32.

Significant Changes

  1. Timer code is updated to set priority through the framework
  2. SoftwareSerial is now used through the framework
  3. Compiler arguments are simplified for USB
    • VID/PID is unchanged, it just occurs by default rather than being explicitly defined
    • Product string defaults to PlatformIO environment name, rather than being explicitly defined
  4. FYSETC S6 board/variant is renamed to differ from framework-provided variant
    • The default variant did not work properly for fan PWM
    • Custom script is eliminated, using generic variant creation script instead.
    • It is possible other boards/variants will need changes if fan PWM does not work correctly
  5. Malyan M300 now extends from common_stm32.

What have I tested?

  • FYSETC S6 (Prior to me accidentally destroying it)
    • Fan PWM
    • BLTouch/Servo control
    • RepRapFullGraphics LCD
    • SPEAKER plays properly
    • Thermistor Reading/Display
    • Flash EEPROM Emulation
  • SKR Pro V1.1
    • Fan PWM
    • BLTouch/Servo control
    • RepRapFullGraphics LCD
    • SoftwareSerial with a TMC2208 driver
    • Thermistor Reading/Display
    • Flash EEPROM Emulation
    • Verified proper STEP/TEMP/SoftwareSerial ISR Preemption
  • GTR V1.0
    • Fan PWM
    • I2C EEPROM
    • RepRapFullGraphics LCD
    • SoftwareSerial
    • Thermistor
  • Malyan M200v2
    • Motion works
    • Thermistors had issues on my board, but I suspect a hardware issue

Benefits

Two key benefits:

  1. All HAL/STM32 environments use the same framework version.
  2. Provides building block to begin migrating away from Maple for STM32F1 boards

Related Issues

N/A

@sjasonsmith sjasonsmith marked this pull request as draft July 2, 2020 07:12
@sjasonsmith sjasonsmith added A: STM32 PR: Improvement T: HAL & APIs Topic related to the HAL and internal APIs. labels Jul 2, 2020
@sjasonsmith
Copy link
Contributor Author

I've updated the description to include soem sanity checks I've performed on a GTR V1.0.

@sjasonsmith
Copy link
Contributor Author

I was able to boot on some Malyan board I have. I messed up my bootloader, but managed to get something on there and write the firmware using a DFU connection over USB.

This appears to be an "M200 v1" board, but has an STM32F070CB (Like the M300). I don't think the pinout exactly matches anything in Marlin right now, but I was able to move a motor and verify that endstop pins work, at least.

@Bob-the-Kuhn
Copy link
Contributor

@rudihorn - I think some of your work on feature request 18157 parallels this PR.

@sjasonsmith
Copy link
Contributor Author

I’m hoping that this can go in soon after the next tagged release, so people can start building features on top of the latest framework. 1.8 was full of bugs that had to be worked around, but so far 1.9 seems better.

@xC0000005
Copy link
Contributor

If you have an M200 mainboard and it has an 070 MCU on it, it's a V2. There's a very small number of V2s which have F103 processors on them (and an even smaller, like, possibly a handful) of M300 boards that have an 103. Does it reboot after 8 seconds? If not, it's in decent shape.

@sjasonsmith
Copy link
Contributor Author

sjasonsmith commented Jul 2, 2020

@xC0000005 I have an M200 with an 070CB (I think the one in the platformio is an 070RB).
I also have another with an F103, but I haven't even powered that one on yet. These were used boards I picked up cheap on eBay. I destroyed the original board from my Mini a long time ago.

There is a surprisingly huge number of variants of these boards....

@xC0000005
Copy link
Contributor

xC0000005 commented Jul 2, 2020 via email

@sjasonsmith
Copy link
Contributor Author

sjasonsmith commented Jul 2, 2020

Did they release bootloader source by any chance? I accidentally erased mine, and I'm using someone's open-source bootloader I found on github, but I doubt it is capable of updating firmware from SD.

@xC0000005
Copy link
Contributor

xC0000005 commented Jul 2, 2020 via email

@thisiskeithb
Copy link
Member

I'll get my MK3S/BTT002 build running on this and run some test prints this weekend if the PR isn't merged before then.

@sjasonsmith
Copy link
Contributor Author

I certainly hope this is not merged before the 2.0.6 release, and people test some actual prints with it.

@sjasonsmith
Copy link
Contributor Author

I just instrumented Marlin and the framework's SoftwareSerial for an SKR Pro to verify that the preemption of ISRs is behaving as expected.
The image below shows the STEP ISR preempting the TEMP ISR, and the SoftwareSerial ISR preempting them both.

This was measured by instrumenting each ISR to set output lines high while active, and capturing the behavior on a logic analyzer.

image

@sjasonsmith sjasonsmith added the Needs: Testing Testing is needed for this change label Jul 4, 2020
@GhostlyCrowd
Copy link
Contributor

I've got some hardware changes to do (dual Z stepper and a new lcd) after i get those verified working ill pull this and test on the SKR Pro 1.1 with 5 2209's and BLTouch

@GhostlyCrowd
Copy link
Contributor

GhostlyCrowd commented Jul 4, 2020

Just dropping this in here as per our discussion.

SKR Pro V1.1 no longer passes sanity checks for PWM on extruder auto pans if the speed is set to anything other than 255.

This may be related to STM32 missing

#ifndef digitalPinHasPWM
#define digitalPinHasPWM(P) (PIN_MAP[P].timer_device != nullptr)
#define NO_COMPILE_TIME_PWM
#endif

Which is missing out of STM32 HALL.H, i tried adding it but it still errors so their are deeper issues.

Commenting out static_assert(PWM_PIN(E0_AUTO_FAN_PIN), "E0" AF_ERR_SUFF);

In Marlin/src/inc/sanitycheck.h is a work around

Log Exerpt from VS Code

                 from Marlin\src\HAL\STM32\../shared/Marduino.h:36,
                 from Marlin\src\HAL\STM32\HAL.h:28,
                 from Marlin\src\HAL\STM32\HAL.cpp:25:
C:\Users\JeffJ\.platformio\packages\framework-arduinoststm32@4.10700.200103\cores\arduino/pins_arduino.h:254:51: error: non-constant condition for static assertion
  254 | #define digitalPinHasPWM(p)         (pin_in_pinmap(digitalPinToPinName(p), PinMap_PWM))
      |                                     ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Marlin\src\HAL\STM32\fastio.h:83:33: note: in expansion of macro 'digitalPinHasPWM'
   83 | #define PWM_PIN(P)              digitalPinHasPWM(P)
      |                                 ^~~~~~~~~~~~~~~~
Marlin\src\HAL\STM32\../../inc/SanityCheck.h:2091:19: note: in expansion of macro 'PWM_PIN'
 2091 |     static_assert(PWM_PIN(E0_AUTO_FAN_PIN), "E0" AF_ERR_SUFF);```


@sjasonsmith
Copy link
Contributor Author

@GhostlyCrowd posted a PR for the SanityCheck issue above, so I won't be spending time on that.
#18539

@sjasonsmith
Copy link
Contributor Author

sjasonsmith commented Jul 4, 2020

There are at least two issues on the Rumba32.

  1. Fan PWM is running at only about 38 Hz after I disable FAN_SOFT_PWM
    • Commenting out PWM_FREQUENCY in variant.h allows it to work at 1 kHz (like the SKR Pro)
    • Leaving PWM_FREQUENCY set to 20000 is not working (even though I think it does on the FYSETC S6)
    • I thought I had it working at 20kHz prior to the official 1.9 framework release. Maybe one of the recent changes by @chrissbarr will require a little extra rework with the new framework.
  2. SERIAL_TIMER is overridden in the pins file, but that is no longer possible
    • Will need to be overridden in variants.h or platformio.ini
    • A static assert could hopefully be added to the pins file

I haven't done much else with the Rumba32. The following seem to work:

  • RepRapDiscount full graphics display
  • SPEAKER
  • SD card access on the display
  • Thermistor reading (indicates temp ISR is running)

@sjasonsmith
Copy link
Contributor Author

Actually, I take back my comment that I thought I had fan PWM working at 20 kHz earlier on the Rumba32. When I look at my comments on #17519 I am only very clear that I had it working at 1Khz.

@sjasonsmith
Copy link
Contributor Author

SERIAL_TIMER is overridden in the pins file, but that is no longer possible
Will need to be overridden in variants.h or platformio.ini

This might be a significant issue for boards such as the Rumba32, which are commonly used inside the Arduino IDE. We can more readily control how things build in PlatformIO. I am no expert at building in Arduino, but I assume people would have to go hand edit board definitions or variants to override the default behavior.

The good news is that I have been able to build and upload firmware easily from PlatformIO, at least for my MKS Rumba32. I assume it would work the same for the original Aus3D version.

@GhostlyCrowd
Copy link
Contributor

SERIAL_TIMER is overridden in the pins file, but that is no longer possible
Will need to be overridden in variants.h or platformio.ini

This might be a significant issue for boards such as the Rumba32, which are commonly used inside the Arduino IDE. We can more readily control how things build in PlatformIO. I am no expert at building in Arduino, but I assume people would have to go hand edit board definitions or variants to override the default behavior.

The good news is that I have been able to build and upload firmware easily from PlatformIO, at least for my MKS Rumba32. I assume it would work the same for the original Aus3D version.

I think its about time Marlin drops support for Arduino IDE in my opinion, we have to move forward eventually and it has a laundry list of issues on a good day. their are multiple options to use PIO and VS code is very easy to use for example.

@xC0000005
Copy link
Contributor

xC0000005 commented Jul 4, 2020 via email

@sjasonsmith
Copy link
Contributor Author

@xC0000005 what is the include path needed for? I had originally added that for SoftwareSerial (which is now gone). Lerdge must need something else?

Feel free to post a PR to my branch if you have any changes that should be included prior to this merging. If it is something that is already broken in the bugfix branch then it could just as easily be a separate PR later.

@xC0000005
Copy link
Contributor

xC0000005 commented Jul 4, 2020 via email

@sjasonsmith
Copy link
Contributor Author

@thinkyhead how would you like to proceed with this?

I'm not sure whether you would plan to squash this in, or merge it with its commit history. If you aren't squashing it I can rebase it and consolidate the changes into a more coherent set of incremental changes.

@sjasonsmith sjasonsmith requested a review from thinkyhead August 14, 2020 03:04
@sjasonsmith
Copy link
Contributor Author

After @GhostlyCrowd's report of some BLTouch issues I just did 200 probes without any issues on an SKR Pro. I still suspect perhaps a wiring/connector issue on their printer, unless there is some issue that occurs at power-on that prevents the BLTouch from working?

@GhostlyCrowd
Copy link
Contributor

GhostlyCrowd commented Aug 16, 2020

After @GhostlyCrowd's report of some BLTouch issues I just did 200 probes without any issues on an SKR Pro. I still suspect perhaps a wiring/connector issue on their printer, unless there is some issue that occurs at power-on that prevents the BLTouch from working?

It hasn't happened since i replaced the wiring and the pin as it was bent badly after the last crash, to be safe not sure, what happened but its safe to say its nothing related to this pr

@GhostlyCrowd
Copy link
Contributor

All is well here. Still think this is ready to merge. My issues were my own. Did a repeatability with 200 probes fine. UBL built and verified a mesh fine and I just completed a 16 hour print.

Once this is merged ill test your other PR @sjasonsmith, but Ii don't want to have any regressions since this printer on the SKR Pro is running so beautifully now I have to wait for this to be merged into bugfix.

@sjasonsmith
Copy link
Contributor Author

@thinkyhead, I've rebased this on top of the latest bugfix-2.0.x and reworked the commit history to give you a concise sequence of changes to look at.

I did some sanity checks on a Malyan M200v2 (STM32F070CB) and verified that at least some of the motors, thermistors, and endstops are working. I didn't test everything due to some limitations testing on my bench.

@sjasonsmith
Copy link
Contributor Author

I cherry-picked the LCD change from #19297 so that CI can run.

@sjasonsmith sjasonsmith removed the Needs: Testing Testing is needed for this change label Sep 7, 2020
Copy link
Member

@thinkyhead thinkyhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ready to test, and there are not too many relevant changes to sift through if problems do arise.

@thinkyhead thinkyhead merged commit 4fc1aba into MarlinFirmware:bugfix-2.0.x Sep 8, 2020
thinkyhead pushed a commit to thinkyhead/Marlin that referenced this pull request Sep 9, 2020
davidveg added a commit to davidveg/Marlin that referenced this pull request Sep 9, 2020
* commit 'bc7720c0cd3917f44200c0b78e1b635e4c7b6797': (483 commits)
  Minor HAL cleanup
  [cron] Bump distribution date (2020-09-09)
  Update HAL/STM32 platform to 8.0 (MarlinFirmware#18496)
  Make M600 heat up the nozzle. Reset runout on fail. (MarlinFirmware#19298)
  [cron] Bump distribution date (2020-09-08)
  TFT is neither "graphical" nor "character" (MarlinFirmware#19297)
  Sanity-check BABYSTEP_DISPLAY_TOTAL with ColorUI (MarlinFirmware#19284)
  Fix M166 Gradient Mix for DELTA (MarlinFirmware#19285)
  Separate Neopixel followup (MarlinFirmware#19287)
  Clean up LCD conditionals, DWIN
  Whitespace cleanup
  Adjust GTR PeripheralPins to avoid timer conflicts (MarlinFirmware#19183)
  STM32F1 EP with USB_COMPOSITE (MarlinFirmware#19281)
  Menu items for Separate NeoPixel (MarlinFirmware#19280)
  [cron] Bump distribution date (2020-09-07)
  Clarify disabling StallGuard for axes (MarlinFirmware#19263)
  Touch UI long filenames fixes (MarlinFirmware#19262)
  Fix Ender 3 V2 (DWIN) buffer overrun (MarlinFirmware#19268)
  Fix STM32F1 SPI device init, MKS_LCD12864 (MarlinFirmware#19271)
  Emergency Parser for STM32F1 (MarlinFirmware#19279)
  ...

# Conflicts:
#	.github/issue_template.md
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
#	README.md
davidveg added a commit to davidveg/Marlin that referenced this pull request Sep 9, 2020
* commit 'bc7720c0cd3917f44200c0b78e1b635e4c7b6797': (483 commits)
  Minor HAL cleanup
  [cron] Bump distribution date (2020-09-09)
  Update HAL/STM32 platform to 8.0 (MarlinFirmware#18496)
  Make M600 heat up the nozzle. Reset runout on fail. (MarlinFirmware#19298)
  [cron] Bump distribution date (2020-09-08)
  TFT is neither "graphical" nor "character" (MarlinFirmware#19297)
  Sanity-check BABYSTEP_DISPLAY_TOTAL with ColorUI (MarlinFirmware#19284)
  Fix M166 Gradient Mix for DELTA (MarlinFirmware#19285)
  Separate Neopixel followup (MarlinFirmware#19287)
  Clean up LCD conditionals, DWIN
  Whitespace cleanup
  Adjust GTR PeripheralPins to avoid timer conflicts (MarlinFirmware#19183)
  STM32F1 EP with USB_COMPOSITE (MarlinFirmware#19281)
  Menu items for Separate NeoPixel (MarlinFirmware#19280)
  [cron] Bump distribution date (2020-09-07)
  Clarify disabling StallGuard for axes (MarlinFirmware#19263)
  Touch UI long filenames fixes (MarlinFirmware#19262)
  Fix Ender 3 V2 (DWIN) buffer overrun (MarlinFirmware#19268)
  Fix STM32F1 SPI device init, MKS_LCD12864 (MarlinFirmware#19271)
  Emergency Parser for STM32F1 (MarlinFirmware#19279)
  ...

# Conflicts:
#	.github/issue_template.md
#	README.md
davidveg added a commit to davidveg/Marlin that referenced this pull request Sep 9, 2020
* 2.0.x: (483 commits)
  Minor HAL cleanup
  [cron] Bump distribution date (2020-09-09)
  Update HAL/STM32 platform to 8.0 (MarlinFirmware#18496)
  Make M600 heat up the nozzle. Reset runout on fail. (MarlinFirmware#19298)
  [cron] Bump distribution date (2020-09-08)
  TFT is neither "graphical" nor "character" (MarlinFirmware#19297)
  Sanity-check BABYSTEP_DISPLAY_TOTAL with ColorUI (MarlinFirmware#19284)
  Fix M166 Gradient Mix for DELTA (MarlinFirmware#19285)
  Separate Neopixel followup (MarlinFirmware#19287)
  Clean up LCD conditionals, DWIN
  Whitespace cleanup
  Adjust GTR PeripheralPins to avoid timer conflicts (MarlinFirmware#19183)
  STM32F1 EP with USB_COMPOSITE (MarlinFirmware#19281)
  Menu items for Separate NeoPixel (MarlinFirmware#19280)
  [cron] Bump distribution date (2020-09-07)
  Clarify disabling StallGuard for axes (MarlinFirmware#19263)
  Touch UI long filenames fixes (MarlinFirmware#19262)
  Fix Ender 3 V2 (DWIN) buffer overrun (MarlinFirmware#19268)
  Fix STM32F1 SPI device init, MKS_LCD12864 (MarlinFirmware#19271)
  Emergency Parser for STM32F1 (MarlinFirmware#19279)
  ...

# Conflicts:
#	Marlin/Configuration.h
#	platformio.ini
@sjasonsmith sjasonsmith deleted the PR/STM32_1.9 branch November 23, 2020 09:35
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
kageurufu pushed a commit to CR30-Users/Marlin-CR30 that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: STM32 PR: Improvement T: HAL & APIs Topic related to the HAL and internal APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants