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

Allow STM32 pins to specify timers #17805

Merged
merged 7 commits into from
May 1, 2020

Conversation

xC0000005
Copy link
Contributor

Description

Set timer overrides for the M200 V1 and V2.

Benefits

Prevents people from having to do this manually.

Related Issues

@randellhodges
Copy link
Contributor

@xC0000005
Copy link
Contributor Author

Indeed it is. But, put a #error after the #ifndef and you'll find it's not effective for some reason. The STM32F1 hal has similar overrides. I'm working on code to autoselect timers, as this isn't something people should maintain, but for now making it at least work without changes is better than nothing.

@sjasonsmith
Copy link
Contributor

The timer header is included very early, long before the pins file, so declaring timers in the pins file isn’t used. They should probably be removed from any pins files that have them, since it gives the false impression that it is handled.

@xC0000005, I’m curious to see what you come up with for auto assignment. STM32 timer conflicts are a pain.

@xC0000005
Copy link
Contributor Author

xC0000005 commented Apr 30, 2020 via email

@randellhodges
Copy link
Contributor

Interesting, yeah, that #define did give me a false impression. Assuming auto select doesn't work out well, would the better place for this be in one of the variant files, or is putting it in the timers.h the only real option.

Not trying to hijack the PR, just trying to get a good understand of the how and why :)

@sjasonsmith
Copy link
Contributor

@xC0000005 your approach seems promising. A couple things I noticed are that I believe heaters always use software PWM. There can also be some timers used in the frameworks by SoftwareSerial, Servo, and Tone(?).

I’ve noticed that the STM32F1 framework disabled any timers attached to SPI pins. I’m not sure if they actually interfere, or if they are being by overly-cautious.

@xC0000005
Copy link
Contributor Author

xC0000005 commented Apr 30, 2020 via email

@xC0000005
Copy link
Contributor Author

xC0000005 commented Apr 30, 2020 via email

@thinkyhead
Copy link
Member

We have control over the order of defines. As long as a timers.h file includes inc/MarlinConfig.h or pins/pins.h at the top, the pins file will be able override the defines that follow. So we do not have to use the MB macro inside of the HAL.

@sjasonsmith
Copy link
Contributor

We have control over the order of defines. As long as a timers.h file includes inc/MarlinConfig.h or pins/pins.h at the top, the pins file will be able override the defines that follow. So we do not have to use the MB macro inside of the HAL.

I believe you currently end up with circular includes, due to timer.h being needed inside the HAL's main function. I'm not saying it isn't solvable, but there is work to be done to make it happy. If the timer numbers were moved into timer.cpp it would be trivial to override them with values from the pins file.

@thinkyhead
Copy link
Member

Ok, I've fixed up the code so pins files can override the timers. Did you want both M200 and M300 to use those pins, or are the overrides in the pins files already correct?

@thinkyhead
Copy link
Member

thinkyhead commented May 1, 2020

I believe you currently end up with circular includes

Fortunately, with #pragma once or #ifdef barriers, includes behave sensibly and "circular" includes are bypassed in favor of "include whatever can be included at this spot." Each file ends up being included just once. The include order ends up being the same as if you took the full includes hierarchy in its nested order, left-justified it, and stripped out the duplicates.

@thinkyhead thinkyhead added A: STM32 C: Boards/Pins PR: Improvement T: HAL & APIs Topic related to the HAL and internal APIs. labels May 1, 2020
@thinkyhead thinkyhead merged commit b4aebbe into MarlinFirmware:bugfix-2.0.x May 1, 2020
@thinkyhead thinkyhead changed the title Set Timer Overrides for M200 V1 and V2. Allow STM32 pins to specify timers May 1, 2020
@GhostlyCrowd
Copy link
Contributor

GhostlyCrowd commented May 1, 2020

This merge breaks compiling for TMC 2209's on the STM32F4 platform and I'm assuming anything else that uses SW serial? reverting this commit solves this issue

Marlin\src\HAL\STM32\HAL.cpp: In function 'void HAL_init()': Marlin\src\HAL\STM32\HAL.cpp:86:42: error: 'SWSERIAL_TIMER_IRQ_PRIO' was not declared in this scope SoftwareSerial::setInterruptPriority(SWSERIAL_TIMER_IRQ_PRIO, 0); ^~~~~~~~~~~~~~~~~~~~~~~ Marlin\src\HAL\STM32\HAL.cpp:86:42: note: suggested alternative: 'EXTI_IRQ_PRIO' SoftwareSerial::setInterruptPriority(SWSERIAL_TIMER_IRQ_PRIO, 0); ^~~~~~~~~~~~~~~~~~~~~~~ EXTI_IRQ_PRIO *** [.pio\build\BIGTREE_SKR_PRO\src\src\HAL\STM32\HAL.cpp.o] Error 1

HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
@xC0000005 xC0000005 deleted the M200QualityOfLife branch October 4, 2022 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: STM32 C: Boards/Pins 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.

5 participants