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.

Sign up for GitHub

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

Jump to bottom

Refactor HAL as singleton #23295

Merged
merged 12 commits into from
Dec 25, 2021
Merged

Refactor HAL as singleton #23295

merged 12 commits into from
Dec 25, 2021

Conversation

thinkyhead
Copy link
Member

Start to wrap HAL methods in a simple MarlinHAL class for the sake of encapsulation. Since HAL Timer methods are also a core part of hardware, like interrupts generally, ADC, and PWM, the HAL timer functions may also get added to this class.

@thinkyhead thinkyhead added PR: Coding Standards PR: General Cleanup T: HAL & APIs Topic related to the HAL and internal APIs. labels Dec 13, 2021
@thinkyhead thinkyhead force-pushed the bf2_hal_refactor_PR branch 3 times, most recently from 9bcae4b to c8f377e Compare December 13, 2021 04:47
@thinkyhead thinkyhead marked this pull request as draft December 13, 2021 04:47
@thinkyhead thinkyhead force-pushed the bf2_hal_refactor_PR branch 12 times, most recently from 239660c to 2534e4e Compare December 13, 2021 08:10
@thinkyhead thinkyhead requested a review from p3p December 13, 2021 08:25
@thinkyhead thinkyhead force-pushed the bf2_hal_refactor_PR branch 5 times, most recently from 9c74c51 to b36628a Compare December 13, 2021 09:37
@thinkyhead thinkyhead marked this pull request as ready for review December 14, 2021 06:24
@thinkyhead
Copy link
Member Author

@p3p — This is going to affect https://github.com/p3p/MarlinSimUI/blob/master/src/MarlinSimulator/marlin_hal_impl/HAL.cpp but it looks like not much else there, so I can submit a patch for MarlinSimUI soon to go along with this.

thinkyhead added a commit to thinkyhead/MarlinSimUI that referenced this pull request Dec 14, 2021
@thinkyhead
MarlinHAL 2.0.9.3
abc70b4 
See MarlinFirmware/Marlin#23295
@thinkyhead thinkyhead mentioned this pull request Dec 14, 2021
Merged
@thinkyhead
Copy link
Member Author

thinkyhead commented Dec 25, 2021 •
edited
Loading

I'm very familiar with CRTP. (See #21344 #20959 #20913 #20783.) You'll find CRTP used or at least approached in the L6470 and TMCStepper libraries. If you want to make a run at applying CRTP as a way of defining an "abstract class" (also called "protocols" in Objective-C) that might be a good help to make sure new HALs are compliant and easier to implement. Between now and then, I still want to look at incorporating all the HAL timer methods into the class, as well.

@thinkyhead thinkyhead merged commit e211ff1 into MarlinFirmware:bugfix-2.0.x Dec 25, 2021
@thinkyhead thinkyhead deleted the bf2_hal_refactor_PR branch December 25, 2021 03:34
thinkyhead added a commit that referenced this pull request Dec 25, 2021
@thinkyhead
♻️ Refactor HAL as singleton (#23295)
4aba367
thisiskeithb pushed a commit to thisiskeithb/Marlin that referenced this pull request Dec 25, 2021
@thinkyhead @thisiskeithb
Revert "♻️ Refactor HAL as singleton (MarlinFirmware#23295)"
02f3679 
This reverts commit e211ff1.
thisiskeithb pushed a commit to thisiskeithb/Marlin that referenced this pull request Dec 25, 2021
@thinkyhead @thisiskeithb
Revert "♻️ Refactor HAL as singleton (MarlinFirmware#23295)"
f30e6cf 
This reverts commit e211ff1.
@thisiskeithb
Copy link
Member

This change prevents my BTT002 & SKR 2 builds from booting. There's also a report on Discord for RAMPS. I suspect this affects all builds, but I'll work through my other machines/boards later today.

@robbycandra
Copy link
Contributor

@thisiskeithb , confirm on SKR 2

@thisiskeithb
Copy link
Member

Filed as an issue:

thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
🐛 Fix freeMemory endless loop
4f0932e 
Followup to #23295
thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
🐛 Fix freeMemory endless loop
1fc66aa 
Followup to #23295
@robbycandra robbycandra mentioned this pull request Dec 26, 2021
Merged
thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
♻️ Refactor HAL as singleton (#23295, #23354)
75337cc
thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
🐛 Fix adc_start for AVR, native
00e6e90 
Followup to #23295
thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
♻️ Refactor HAL as singleton (#23295, #23354)
059ae4d
thinkyhead added a commit that referenced this pull request Dec 26, 2021
@thinkyhead
⏪️ Refactor still needs work
6a8b927 
Reverting #23295
Foxies-CSTL added a commit to Foxies-CSTL/Marlin_2.1.x that referenced this pull request Dec 26, 2021
@Foxies-CSTL
Squashed commit of the following:
d229168 
commit 1a6dbcb
Merge: eebf187 56ac681
Author: Foxies-CSTL <Foxies-CSTL@users.noreply.github.com>
Date:   Sun Dec 26 19:02:32 2021 +0100

    Merge remote-tracking branch 'upstream/bugfix-2.0.x' into bugfix-2.0.x

commit 56ac681
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sun Dec 26 03:20:29 2021 -0600

    🎨 Pins and SDIO cleanup

commit 7762df7
Author: EvilGremlin <22657714+EvilGremlin@users.noreply.github.com>
Date:   Sun Dec 26 09:46:13 2021 +0300

    🔧 Check Chiron LCD requirements (MarlinFirmware#23353)

    Co-Authored-By: EvilGremlin <22657714+EvilGremlin@users.noreply.github.com>

commit 6a8b927
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 23:15:17 2021 -0600

    ⏪️ Refactor still needs work

    Reverting MarlinFirmware#23295

commit 00e6e90
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 22:10:47 2021 -0600

    🐛 Fix adc_start for AVR, native

    Followup to MarlinFirmware#23295

commit 555c749
Author: Keith Bennett <13375512+thisiskeithb@users.noreply.github.com>
Date:   Sat Dec 25 19:41:01 2021 -0800

    🐛 Fix MKS Robin E3 NeoPixel pin default (MarlinFirmware#23350)

commit a47f559
Author: Robby Candra <robbycandra.mail@gmail.com>
Date:   Sun Dec 26 10:36:09 2021 +0700

    🐛 HAL refactor followup (MarlinFirmware#23354)

commit 57315f0
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 21:25:47 2021 -0600

    🐛 Fix missing ADC method

commit d7af619
Author: kaidegit <60053077+kaidegit@users.noreply.github.com>
Date:   Sun Dec 26 10:12:20 2021 +0800

    🐛 Fix open for bin rename (MarlinFirmware#23351)

commit 4f0932e
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 20:00:48 2021 -0600

    🐛 Fix `freeMemory` endless loop

    Followup to MarlinFirmware#23295

commit bdb0716
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Sun Dec 26 01:09:46 2021 +0000

    [cron] Bump distribution date (2021-12-26)

commit fca6d12
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 03:27:45 2021 -0600

    🔧 Move MOTHERBOARD closer to top

commit c74161c
Author: fflosi <34758322+fflosi@users.noreply.github.com>
Date:   Sat Dec 25 05:57:07 2021 -0300

    ✨ Per-axis TMC hold multiplier (MarlinFirmware#23345)

commit a941cd3
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 23:47:52 2021 -0600

    🔨 Configurable firmware bin filename

    Configuration.h > FIRMWARE_BIN

commit e211ff1
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 21:33:59 2021 -0600

    ♻️ Refactor HAL as singleton (MarlinFirmware#23295)

commit 532f21f
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 21:32:00 2021 -0600

    🔨 Ignore sublime workspace file

commit cd1920b
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 20:59:28 2021 -0600

    🔨 Ignore cmake generated build folder

commit 6d09d1d
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Sat Dec 25 01:04:01 2021 +0000

    [cron] Bump distribution date (2021-12-25)

commit d5dff19
Author: Scott Lahteine <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 01:46:51 2021 -0600

    🔧 Sanity check MMU2_MENUS

commit 21cd715
Author: Sola <42537573+solawc@users.noreply.github.com>
Date:   Fri Dec 24 14:03:32 2021 +0800

    ✨ MKS TinyBee board support (MarlinFirmware#23340)

    Co-Authored-By: Sola <42537573+solawc@users.noreply.github.com>

commit 80a537c
Author: Attila BODY <attila.body@gmail.com>
Date:   Fri Dec 24 06:57:20 2021 +0100

    🐛 Fix Robin Nano v3 filament runout pins (MarlinFirmware#23344)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit ca0215b
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Fri Dec 24 01:04:40 2021 +0000

    [cron] Bump distribution date (2021-12-24)

commit df9eb56
Author: MrAlvin <umo-testing@3iii.dk>
Date:   Thu Dec 23 10:47:52 2021 +0100

    🚸 Show mm'ss during first hour (MarlinFirmware#23335)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit 3e737cb
Author: Robby Candra <robbycandra.mail@gmail.com>
Date:   Thu Dec 23 15:19:39 2021 +0700

    🔧 Group FAST_PWM_FAN.options (MarlinFirmware#23331)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit da67deb
Author: Tanguy Pruvot <tpruvot@users.noreply.github.com>
Date:   Thu Dec 23 08:32:27 2021 +0100

    ✏️ Fix missing brace (MarlinFirmware#23337)

    Followup to MarlinFirmware#22916

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit 1b876c1
Author: Tanguy Pruvot <tpruvot@users.noreply.github.com>
Date:   Thu Dec 23 07:49:15 2021 +0100

    🩹 Fix settings G21 report (MarlinFirmware#23338)

commit c80ef71
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Thu Dec 23 01:05:31 2021 +0000

    [cron] Bump distribution date (2021-12-23)

commit 2893048
Author: InsanityAutomation <38436470+InsanityAutomation@users.noreply.github.com>
Date:   Tue Dec 21 23:09:55 2021 -0500

    ✨ BLTouch High Speed mode runtime configuration (MarlinFirmware#22916)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit c1dba3d
Author: ellensp <530024+ellensp@users.noreply.github.com>
Date:   Wed Dec 22 15:44:04 2021 +1300

    ✨ Option to reset EEPROM on first run (MarlinFirmware#23276)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit fdd37a8
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Wed Dec 22 01:07:10 2021 +0000

    [cron] Bump distribution date (2021-12-22)

commit 9cee626
Author: ellensp <530024+ellensp@users.noreply.github.com>
Date:   Wed Dec 22 13:48:38 2021 +1300

    🐛 Fix Chitu Z_STOP_PIN (MarlinFirmware#23330)

commit f374fa0
Author: Mike La Spina <mike.laspina@shaw.ca>
Date:   Tue Dec 21 01:26:31 2021 -0600

    🚑️ FAST_PWM_FAN default 1KHz base freq. (MarlinFirmware#23326)

    Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>

commit b06f871
Author: thinkyhead <thinkyhead@users.noreply.github.com>
Date:   Tue Dec 21 01:08:00 2021 +0000

    [cron] Bump distribution date (2021-12-21)
p3p pushed a commit to p3p/MarlinSimUI that referenced this pull request Dec 27, 2021
@thinkyhead
MarlinHAL 2.0.9.3 (#9)
15d05a0 
See MarlinFirmware/Marlin#23295
@thinkyhead thinkyhead mentioned this pull request Jan 26, 2022
Merged
11 tasks
mh-dm pushed a commit to mh-dm/Marlin that referenced this pull request May 15, 2022
@thinkyhead @mh-dm
♻️ Refactor HAL as singleton (MarlinFirmware#23295)
0ba533c
mh-dm pushed a commit to mh-dm/Marlin that referenced this pull request May 15, 2022
@thinkyhead @mh-dm
🐛 Fix freeMemory endless loop
0c4246a 
Followup to MarlinFirmware#23295
mh-dm pushed a commit to mh-dm/Marlin that referenced this pull request May 15, 2022
@thinkyhead @mh-dm
🐛 Fix adc_start for AVR, native
8529e10 
Followup to MarlinFirmware#23295
mh-dm pushed a commit to mh-dm/Marlin that referenced this pull request May 15, 2022
@thinkyhead @mh-dm
⏪️ Refactor still needs work
02735ff 
Reverting MarlinFirmware#23295
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
PR: Coding Standards PR: General Cleanup T: HAL & APIs Topic related to the HAL and internal APIs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thinkyhead @sjasonsmith @thisiskeithb @robbycandra