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

Onboard (always-on) pullups support #16144

Merged
merged 2 commits into from
Dec 12, 2019

Conversation

ManuelMcLure
Copy link
Contributor

@ManuelMcLure ManuelMcLure commented Dec 8, 2019

Allow specifying that a board has onboard pull-ups to satisfy sanity checks.

Description

Some modern boards such as Re-ARM or the BigTreetech SKR 1.3 have built in pull-ups, but there is no way to tell Marlin this. This causes some sanity checks to fail and forces users to enable internal pull-ups even though they are not necessary.
This fix adds a new ONBOARD_ENDSTOPPULLUPS option that can be defined in a pins file that will satisfy the sanity check and not require having the internal pull-up in parallel with the onboard one.
It also enables this option in the Re-ARM and BTT SKR 1.3 pins files.

Benefits

Allows using boards with onboard pull-ups without also setting ENDSTOPPULLUPS in cases where a pull-up is needed.

Allow specifying that a board has onboard pullups to satisfy sanity checks.

Allow specifying that board has pull-ups.
@thinkyhead
Copy link
Member

thinkyhead commented Dec 8, 2019

It may be better to just enable ENDSTOPPULLUPS in Conditionals.h for certain boards / platforms, or set the individual endstop options based on which pin they're assigned to. That way if an endstop is reassigned to some pin that doesn't have the always-on pullup, then the behavior can adapt.

Along the same line, instead of setting pullups on pins that have them always-on, we should detect that and do nothing.

@ManuelMcLure
Copy link
Contributor Author

Isn’t setting ENDSTOPPULLUPS in Conditionals.h going to have the same effect as setting it in the configuration, i.e. activating the internal pull-up? That’s exactly what we don’t want - this is a way to say “endstops on this board already have pull-ups so don’t complain about the user not explicitly enabling them.”

@ManuelMcLure
Copy link
Contributor Author

I just noticed that my description can be interpreted wrong - I’ve updated it to make it clearer.

@thinkyhead
Copy link
Member

thinkyhead commented Dec 11, 2019

It's an open question how to handle it. If a pin's pullup can't be turned off, then it may make sense to both:

  • require the pullup option to be enabled for the pin in the config, and
  • leave the pin in its natural pulled up state.

In this way the configuration (when read later) accurately expresses the state of the pullup in relation to the endstop. This still requires no change to configs, and is still handled transparent to the user in the conditionals or the sanity-check, but adds the extra burden for us to know more about each pin.

The main (maybe minor) issue I'm concerned about is that the pin might be edited, and the new pin might not have the pullup, and it would be nice to be able to handle that transparently in some way, basically just by changing how the pin is initialized.

This becomes more important if we later allow pin assignments to be changed at runtime (like RRF).

@ManuelMcLure
Copy link
Contributor Author

Usually for this type of board the endstop pins have extra circuitry (a couple of resistors and a cap) that makes them unusable for other features without surgery with a soldering iron, so I think it's pretty unlikely that other pins on the board will be used for endstops. However, if they do use a different pin they can still explicitly set ENDSTOPPULLUP_* on that pin to get the internal pullup.

If I understand your comment correctly, you want code like this in endstops.cpp:

 166   #if HAS_X_MAX
 167     #if ENABLED(ENDSTOPPULLUP_XMAX) && DISABLED(ONBOARD_ENDSTOPPULLUPS)
 168       SET_INPUT_PULLUP(X_MAX_PIN);
 169     #elif ENABLED(ENDSTOPPULLDOWN_XMAX)
 170       SET_INPUT_PULLDOWN(X_MAX_PIN);
 171     #else
 172       SET_INPUT(X_MAX_PIN);
 173     #endif
 174   #endif

instead of just disabling the sanity check? I think that ends up being less flexible, unless we add ONBOARD_ENDSTOPPULLUP_XMIN and such.

@thinkyhead thinkyhead merged commit b065a60 into MarlinFirmware:bugfix-2.0.x Dec 12, 2019
@thinkyhead thinkyhead changed the title Support onboard pull-ups. Onboard (always-on) pullups support Dec 12, 2019
@ManuelMcLure ManuelMcLure deleted the OnboardPullups branch December 12, 2019 21:55
bjh22 added a commit to bjh22/Marlin that referenced this pull request Dec 15, 2019
* bugfix-2.0.x: (1131 commits)
  TOUCH_MI_DEPLOY_XPOS fallback to X_MIN_POS (MarlinFirmware#16226)
  Fix bad #ifdef (MarlinFirmware#16227)
  Move status screen defines
  Improve A20M config
  Tweak some config names
  Cardreader read/write open methods
  Include Z in SCARA steps feedrate (MarlinFirmware#16193)
  Update M503 MBL G29 report (MarlinFirmware#16199)
  Fix compile error (macro substitution typo) (MarlinFirmware#16194)
  Add MKS Robin Mini EEPROM defines (MarlinFirmware#16203)
  Flsun QQ-S example config (MarlinFirmware#16204)
  Fix RAMBo CNC test
  Tweak ExtUI Probeless Babystepping (MarlinFirmware#16177)
  Onboard (always-on) pullups support (MarlinFirmware#16144)
  Add a CI test for RAMBo + CNC (MarlinFirmware#16126)
  Disable SD_CHECK_AND_RETRY in BTT E3 configs (MarlinFirmware#16143)
  Invert E dir of Geeetech A10 (MarlinFirmware#16149)
  MKS 12864 OLED pins for SGEN-L (MarlinFirmware#16188)
  Update BTT comments for USB/SD Composite (MarlinFirmware#16130)
  Use error message !! hints (MarlinFirmware#16145)
  ...

# Conflicts:
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
#	Marlin/src/pins/lpc1768/pins_BTT_SKR_V1_3.h
#	platformio.ini
LinoBarreca pushed a commit to LinoBarreca/Marlin that referenced this pull request Dec 18, 2019
philippniethammer pushed a commit to philippniethammer/Marlin that referenced this pull request Dec 21, 2019
christran206 pushed a commit to christran206/Marlin2.0-SKR-Mini-E3-1.2 that referenced this pull request Dec 30, 2019
webs1821 added a commit to webs1821/Marlin that referenced this pull request Jan 2, 2020
* [cron] Bump distribution date

* STM32F1 Flash-based EEPROM fixes (MarlinFirmware#16118)

* Disable PRINTCOUNTER in SKR Mini E3 examples (MarlinFirmware#16110)

* Fix compile error with disabled PIDTEMP (MarlinFirmware#16108)

* Wanhao D6 uses TINYBOY2 (MarlinFirmware#16117)

* Improve touch buttons behavior (MarlinFirmware#16109)

* Update AZSMZ LCD link (MarlinFirmware#16106)

* [cron] Bump distribution date

* Update 3DFabXYZ settings (MarlinFirmware#16139)

* [cron] Bump distribution date

* Fix controller and SD on Robin Nano (MarlinFirmware#16187)

* Correct MKS Robin Mini pins (MarlinFirmware#16178)

* Formalize DAC percent strings (MarlinFirmware#16176)

* Update Italian language (MarlinFirmware#16147)

* Update french (objects, retract...)

* Superscript 3 for mm3

* [cron] Bump distribution date

* Add MKS Robin Pro, MKS Robin Lite3 (MarlinFirmware#16163)

* Fix multiple servos with STM32 (MarlinFirmware#16151)

* Use error message !! hints (MarlinFirmware#16145)

* Update BTT comments for USB/SD Composite (MarlinFirmware#16130)

* MKS 12864 OLED pins for SGEN-L (MarlinFirmware#16188)

* Invert E dir of Geeetech A10 (MarlinFirmware#16149)

* Disable SD_CHECK_AND_RETRY in BTT E3 configs (MarlinFirmware#16143)

* Add a CI test for RAMBo + CNC (MarlinFirmware#16126)

* Onboard (always-on) pullups support (MarlinFirmware#16144)

* Tweak ExtUI Probeless Babystepping (MarlinFirmware#16177)

* Fix RAMBo CNC test

* Flsun QQ-S example config (MarlinFirmware#16204)

* Add MKS Robin Mini EEPROM defines (MarlinFirmware#16203)

* Fix compile error (macro substitution typo) (MarlinFirmware#16194)

* Update M503 MBL G29 report (MarlinFirmware#16199)

* Include Z in SCARA steps feedrate (MarlinFirmware#16193)

* Cardreader read/write open methods

* Tweak some config names

* Improve A20M config

* Move status screen defines

* Fix bad #ifdef (MarlinFirmware#16227)

* TOUCH_MI_DEPLOY_XPOS fallback to X_MIN_POS (MarlinFirmware#16226)

* Fix MKS SGen-L SD detect pin (MarlinFirmware#16224)

* Improve ESP32 HAL (EEPROM, watchdog) (MarlinFirmware#16228)

* Fix G28 debug line, M569 calls (MarlinFirmware#16213)

* Add SKR Mini E3 + Zonestar LCD warning (MarlinFirmware#16214)

* STM32 Touch UI timings, Longer onboard pullups (MarlinFirmware#16219)

* Update BTT002 platform (fixing SD init) (MarlinFirmware#16217)

* Define more FAN pins for GT2560 (MarlinFirmware#16235)

* Tweak E180 config

* Update product links

* [cron] Bump distribution date

* Add dev-2.1.x to 'mfinfo' script

* Move AutoBuildMarlin to its own repo

* [cron] Bump distribution date

* Tweak G34, fix a declaration

* Sanity-check Z_STEPPER_AUTO_ALIGN coordinates (MarlinFirmware#16231)

* Improve SKR mini E3 + Ender 3 settings (MarlinFirmware#16247)

* Tweak code formatting

* Allow TMC2209 to save/restore spreadCycle (MarlinFirmware#16153)

Co-Authored-By: teemuatlut <[email protected]>

* Spindle/Laser pins for RADDS (MarlinFirmware#16119)

* Improve pulse timing and step reliability (MarlinFirmware#16128)

* BigTreeTech SKR v1.4 support (MarlinFirmware#16236)

* Followup to TMC2209 spreadcycle patch

* Remove obsolete TMC2209 comment

* HOME_USING_SPREADCYCLE is obsolete

Co-Authored-By: teemuatlut <[email protected]>

* Use MYSERIAL0 (not SerialUSB) for Malyan LCD

* Followup for step timing (MarlinFirmware#16128)

* Fix broken M100_dump_routine

* Tweak sanity checks

* Update test scripts to error on unknown (MarlinFirmware#16229)

* Kossel Clear configuration (MarlinFirmware#16198)

* Move pins debug condition

* Use Github Actions for CI, date bump (MarlinFirmware#16269)

* Fix HAL_STM32 + Arduino IDE SoftwareSerial conflict (MarlinFirmware#16266)

* Improve Anet A6 config (MarlinFirmware#16280)

* Fix G34 with Z_DUAL_STEPPER_DRIVERS compile (MarlinFirmware#16274)

* Fix planner compile error (MarlinFirmware#16272)

* Fix axis CS sanity check (MarlinFirmware#16271)

* Add Tevo Nereus (w/ Robin Nano) config (MarlinFirmware#16207)

* Don't test certain changes

* Reset runout.ran_out on resume (MarlinFirmware#16230)

* Step timing cleanup and rounding fix (MarlinFirmware#16258)

* Add MRR_ESPA/_ESPE (ESP32) boards (MarlinFirmware#16238)

* Add Ender-5 Pro config (MarlinFirmware#16221)

* Add FLYBOARD (STM32F407ZG) (MarlinFirmware#16257)

* Fix STM32 flush of TX (used by UBL) (MarlinFirmware#16197)

* Flash leveling (for some STM32) (MarlinFirmware#16174)

* Some ESP32 patches (MarlinFirmware#16297)

* MKS SGen-L pins EEBF or EFBF scheme (MarlinFirmware#16296)

* Release version 2.0.0

* Add Rumba32 support for PIO (MarlinFirmware#16202)

* MKS Robin 2 (STM32F407ZE) base support (MarlinFirmware#16270)

* Update Czech language (MarlinFirmware#16305)

* Sync SKR E3 configs (MarlinFirmware#16301)

* Add NOZZLE_AS_PROBE (no probe offsets) (MarlinFirmware#15929)

* Version 2.0.1

* Update build status url

Co-authored-by: Scott Lahteine <[email protected]>
Co-authored-by: randellhodges <[email protected]>
Co-authored-by: Jason Smith <[email protected]>
Co-authored-by: Antti Andreimann <[email protected]>
Co-authored-by: thisiskeithb <[email protected]>
Co-authored-by: Tanguy Pruvot <[email protected]>
Co-authored-by: Luu Lac <[email protected]>
Co-authored-by: Alain Martel <[email protected]>
Co-authored-by: Anders Sahlman <[email protected]>
Co-authored-by: dagorel <[email protected]>
Co-authored-by: Giuliano Zaro <[email protected]>
Co-authored-by: MS1987 <[email protected]>
Co-authored-by: MangaValk <[email protected]>
Co-authored-by: André Kjellstrup <[email protected]>
Co-authored-by: Tobias Schürg <[email protected]>
Co-authored-by: Jeff Eberl <[email protected]>
Co-authored-by: ManuelMcLure <[email protected]>
Co-authored-by: InsanityAutomation <[email protected]>
Co-authored-by: iain MacDonnell <[email protected]>
Co-authored-by: Bo Herrmannsen <[email protected]>
Co-authored-by: Luc <[email protected]>
Co-authored-by: danym21 <[email protected]>
Co-authored-by: chzj333 <[email protected]>
Co-authored-by: Vertabreaker <[email protected]>
Co-authored-by: Moshi Binyamini <[email protected]>
Co-authored-by: salami738 <[email protected]>
Co-authored-by: elementfoundry <[email protected]>
Co-authored-by: teemuatlut <[email protected]>
Co-authored-by: Mario Costa <[email protected]>
Co-authored-by: BigTreeTech <[email protected]>
Co-authored-by: Markus Towara <[email protected]>
Co-authored-by: FLYmaker <[email protected]>
Co-authored-by: ferengi82 <[email protected]>
Co-authored-by: petrzjunior <[email protected]>
webs1821 pushed a commit to webs1821/Marlin that referenced this pull request Jan 2, 2020
griehsler pushed a commit to griehsler/Marlin that referenced this pull request Jan 30, 2020
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.

2 participants