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

Add assign PIN to G-CODE macro feature #18389

Conversation

MoellerDi
Copy link
Contributor

@MoellerDi MoellerDi commented Jun 22, 2020

Description

This PR is based on the great idea and work from @android444 (PR #17586) and based on the communication between him and @thinkyhead.

This PR might not be ready and should be seen as proof of concept.
(it was tested on my own printer and works fine)

Benefits

This PR once completed is enabling the user to assign a PIN to predefined g-code macros by using already existing custom menu option.

Related Issues

no issues found but it's related to PR #17586
maybe related to #7932

@MoellerDi MoellerDi marked this pull request as draft June 22, 2020 20:01
@MoellerDi MoellerDi mentioned this pull request Jun 22, 2020
@android444
Copy link
Contributor

I don't know why ANYONE who has authority does not accept this method.
If there is a pattern how to do it, I will gladly do it.

@thinkyhead
Copy link
Member

Deferred until after the next release.

Marlin/src/MarlinCore.cpp Outdated Show resolved Hide resolved
@sjasonsmith
Copy link
Contributor

What is the state of this? We've certainly had our next release. @MoellerDi do you still consider these changes to be Draft?
I think I will flag this as needing discussion and testing, to see if some other people can try it out.

@sjasonsmith sjasonsmith added Needs: Discussion Discussion is needed Needs: Testing Testing is needed for this change labels Sep 25, 2020
@MoellerDi
Copy link
Contributor Author

@sjasonsmith: I'll work on it this weekend to get it to the latest code base and do some cleanup. Should be ready for testing by Monday.

@android444: as it was your idea; anything from your end to be added or are you ok to proceed with this PR? Are you ok to get this merged (once ready) or do you prefer to further work on your PR #17586 ?

@MoellerDi MoellerDi force-pushed the Marlin-PR/bugfix-2.0.x-PR-assign_PIN_to_G-CODE_macro_feature branch from 6832931 to 3652f24 Compare September 26, 2020 08:15
@MoellerDi
Copy link
Contributor Author

IMHO this PR is now ready for review and testing

@MoellerDi MoellerDi marked this pull request as ready for review September 26, 2020 14:34
@Zezoh
Copy link

Zezoh commented Oct 16, 2020

looking forward to test it, :)

Is it possible to add different scenarios for a single button, to override the limitation of available pins :

long push 3 sec
short push 1 sec
one push
two pushes
etc.

@MoellerDi
Copy link
Contributor Author

looking forward to test it, :)

Is it possible to add different scenarios for a single button, to override the limitation of available pins :

long push 3 sec
short push 1 sec
one push
two pushes
etc.

I'll have a look at that and see what I can do once we are sure the basic functionality works and got tested ;)

…ugfix-2.0.x-PR-assign_PIN_to_G-CODE_macro_feature
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Mar 1, 2021
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Mar 1, 2021
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Mar 1, 2021
thinkyhead and others added 4 commits March 1, 2021 07:52
Co-Authored-By: android444 <24375898+android444@users.noreply.github.com>
@thinkyhead thinkyhead merged commit 7ee9aef into MarlinFirmware:bugfix-2.0.x Mar 1, 2021
@DrumClock
Copy link

DrumClock commented Mar 6, 2021

Hi, @MoellerDi
I wanted to define CUSTOM_USER_BUTTONS but somehow it doesn't work.

CUB

PC10

Where is the mistake?

@DrumClock
Copy link

if I disable the #if condition, it's functional. (BNT_PIN = PC10)

UB2

@ellensp
Copy link
Contributor

ellensp commented Mar 8, 2021

himm, that #if looks wrong.
from macros
#define PIN_EXISTS(PN) (defined(PN##_PIN) && PN##_PIN >= 0)

which means it appends the _PIN to the name, so it should be #if PIN_EXISTS(BUTTON1)

@DrumClock
Copy link

Hi @ellensp
yes #if PIN_EXISTS (BUTTON1) already works

Should I fix all BUTTON conditions?

@ellensp
Copy link
Contributor

ellensp commented Mar 8, 2021

yes, are you going to create PR? I will If you don't.

@DrumClock
Copy link

...unfortunately I can't create PR.

@ellensp
Copy link
Contributor

ellensp commented Mar 8, 2021

ok ill do it.

@ellensp
Copy link
Contributor

ellensp commented Mar 8, 2021

PR to fix this has been created

thinkyhead pushed a commit that referenced this pull request Mar 8, 2021
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
Co-Authored-By: android444 <24375898+android444@users.noreply.github.com>
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
Co-Authored-By: android444 <24375898+android444@users.noreply.github.com>
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
Co-Authored-By: android444 <24375898+android444@users.noreply.github.com>
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead added a commit that referenced this pull request Apr 30, 2021
Co-Authored-By: android444 <24375898+android444@users.noreply.github.com>
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
thinkyhead pushed a commit 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
Needs: Testing Testing is needed for this change PR: New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants