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

[BUG] BOARD_MKS_BASE_16 + FILAMENT_RUNOUT_SENSOR won't compile [SOLVED] #16860

Closed
yet-another-average-joe opened this issue Feb 14, 2020 · 5 comments

Comments

@yet-another-average-joe
Copy link

yet-another-average-joe commented Feb 14, 2020

Bug Description

Marlin 2.0.x bugfix 2.0.4 / 71539bc

BOARD_MKS_BASE_16 + FILAMENT_RUNOUT_SENSOR won't compile :

Gives this error :

Marlin\src\HAL\HAL_AVR\../../inc/SanityCheck.h:695:6: error: #error "FILAMENT_RUNOUT_SENSOR requires FIL_RUNOUT_PIN."
     #error "FILAMENT_RUNOUT_SENSOR requires FIL_RUNOUT_PIN."

My Configurations

Required: Please include a ZIP file containing your Configuration.h and Configuration_adv.h files.

Marlin.zip

Steps to Reproduce

define MOTHERBOARD = BOARD_MKS_BASE_16

Expected behavior: [What you expect to happen]

the preprocessor using the default pin for filament runout sensor (digital 4)

Actual behavior: [What actually happens]

the preprocessor throws an error

Additional Information

workaround : define MOTHERBOARD = BOARD_MKS_BASE_15

(1.5 always worked fine for MKS Base 1.6)

OR

in pins_MKS_BASE_16.h, commenting out :

//#ifndef FIL_RUNOUT_PIN
//  #define FIL_RUNOUT_PIN   -1
//#endif

It seems FIL_RUNOUT_PIN is redefined as -1 somewhere, and upsets sanity check

Can't figure where's the bug : some include order ?

@thisiskeithb
Copy link
Member

thisiskeithb commented Feb 15, 2020

#error "FILAMENT_RUNOUT_SENSOR requires FIL_RUNOUT_PIN."

This behavior was changed in #16806 by @DanielMazurkiewicz & @thinkyhead. There isn't a FIL_RUNOUT_PIN defined in your configs, nor the pins_MKS_BASE_16.h file, so it's disabled.

(1.5 always worked fine for MKS Base 1.6)

The MKS Base 1.5 pins file doesn't undefine the pin automatically, so it's set to 4 (and allows for a successful compile):

// RAMPS 1.4 DIO 4 on the servos connector
#ifndef FIL_RUNOUT_PIN
#define FIL_RUNOUT_PIN 4
#endif

Maybe @DanielMazurkiewicz can explain why the change was needed on the 1.6 pins file & not the 1.5/other BASE boards.

@yet-another-average-joe
Copy link
Author

Thanks for the explanations. But I don't understand the reasons why 1.6 does not" behave" as other RAMPS...

A while ago, I had FIL_RUNOUT_PIN defined in Configuration.h. At this time, I did use Z+ for filament runout sensor. I then stopped doing this, decided to use the default pin D4, and removed the FIL_RUNOUT_PIN definition from Configuration.h : Marlin and me were happy and I completely forgot about it !

Maybe, in Configuration.h comment, it could be good to add :

For MKS Base 1.6 boards you MUST define FIL_RUNOUT_PIN, FIL_RUNOUT2_PIN, etc.

@yet-another-average-joe yet-another-average-joe changed the title [BUG] BOARD_MKS_BASE_16 + FILAMENT_RUNOUT_SENSOR won't compile [BUG] BOARD_MKS_BASE_16 + FILAMENT_RUNOUT_SENSOR won't compile [SOLVED] Feb 15, 2020
@thinkyhead
Copy link
Member

thinkyhead commented Feb 15, 2020

Maybe @DanielMazurkiewicz can explain why the change was needed on the 1.6 pins file & not the 1.5/other BASE boards.

Here is the original proposed change: d9ce0f1
I followed up by reorganizing the related pins files for consistency.

@DanielMazurkiewicz
Copy link
Contributor

@thisiskeithb @yet-another-average-joe

Maybe @DanielMazurkiewicz can explain why the change was needed on the 1.6 pins file & not the 1.5/other BASE boards.

Sorry for late answer, but the answer is simple - the board does not define pin FILAMENT_RUNOUT_SENSOR #16783 . This pin has to be remapped from one of other pins defined on board. Aldo wasn't fully aware of issues with Marlin code refactoring when made that PR. But if there were no issues with that I would really keep clean board pin definitions in one file and any mods/redefinitions in separate file in main directory, similar to configuration.h. That would keep code clean easier to understand and modify and would beautifully fall into HAL idea.

@boelle boelle closed this as completed Jun 20, 2020
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants