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

Case light menu (3rd attempt) #5255

Merged
merged 2 commits into from
Nov 23, 2016
Merged

Case light menu (3rd attempt) #5255

merged 2 commits into from
Nov 23, 2016

Conversation

Kaibob2
Copy link
Contributor

@Kaibob2 Kaibob2 commented Nov 19, 2016

Github seems to be for Pros only :)
Referenced to this #5194 (and this #5243 because i don't understand github) i implemented a (definable) Menu entry on the main screen to toggle the case lights. I had this long before the #define CASE_LIGHT_PIN 4 and always found it extremely useful to be able to switch off the case light from by LCD. The host is shut down most of the time, so if #define CASE_LIGHT_DEFAULT_ON is enabled there is no way to switch of the case light if the printer is printing for 20 hours.

@Kaibob2 Kaibob2 mentioned this pull request Nov 19, 2016
@thinkyhead
Copy link
Member

thinkyhead commented Nov 20, 2016

Github seems to be for Pros only :)

Github helpfully provides great documentation. And Google provides a means to find more. The time spent struggling could be the time spent learning. Just as time spent trying to avoid effort now could be time spent learning to reduce effort forever. I personally recommend http://learngitbranching.js.org/ for a start.

MENU_ITEM(gcode, MSG_LIGHTS_ON, PSTR("M355 S1"));
else
MENU_ITEM(gcode, MSG_LIGHTS_OFF, PSTR("M355 S0"));
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So close! Add a blank line above and restore the blank line after, and we're good!

@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 21, 2016

Like this?

@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 22, 2016

What's wrong here, do I have to change anything before this gets merged?

@ghost
Copy link

ghost commented Nov 23, 2016

Maybe removing unnecessary spaces and squash are needed?

remove unnecessary spaces by text editor, and save commit open a git shell git rebase -i HEAD~7
0001 0002 0003 0004
Then memopad is opend edit and close (save) done. upload to web by git push -f
0005 0006 0007 0008

Result: https://github.com/esenapaj/Marlin/tree/CaseLightMenu

@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 23, 2016

@esenapaj I think i got i right now finally. Many thanks for the great guideline. I guess i wouldn't have made it without your advice. It's so easy...if you know how.

@ghost
Copy link

ghost commented Nov 23, 2016

I'm glad that it works, but I'm sorry but it looks like that it needs fixing.
Unnecessary spaces should has been removed, but blank line is still needed.
And, new line at end ofultralcd.cpp is removed for some reason, it needs to be restored.
Needed altering is this.
https://github.com/esenapaj/Marlin/commit/3aab14521659e0e8b9f5c5edbdc5593f81944bb6

If you are thinking that you aren't sure how to fix it, there is also a way to copy my commit.

git remote add temporary https://github.com/esenapaj/Marlin git fetch temporary done. select "temporary/CaseLightMenu"
0011 0012 0013 0014
click SHA, then it's copied to clipboard return to "CaseLightMenu" git cherry-pick and Ctrl + v done.
0015 0016 0017 0018
git rebase -i HEAD~3 edit and close (save) upload to web by git push -f
0019 0020 0021
git remote rm temporary select "temporary/CaseLightMenu" delete "temporary/CaseLightMenu"
0022 0023 0024

@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 23, 2016

Ay, i think i'm on my way messing it all up completely. But i won't give up. I only have to drive home from work :)

@ghost
Copy link

ghost commented Nov 23, 2016

Nice fighting spirits... Yes, it's still recoverable.
When you use

git reset --hard HEAD~

, top of commit is vanished.

If you want to erase the first three commits,

git reset --hard HEAD~3

.

Or, another way for erasing commits available.

git rebase -i HEAD~5 Then memopad is opend edit and close (save) vanished.
0001 0002 0003 0004

@stigjoergensen
Copy link

@esenapaj I really like all these screen shots... and appreciate them so very much...

Could i ask a huge favor... write them up in a guide on how to use github with marlin and your local fork - it would be a huge help for many i think - and i think @thinkyhead or other could have them in the main wiki for everybody to find easily...

Thanks
/Stig

@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 23, 2016

@esenapaj Not completely sure, but i think i've got it now. fingerscrossed

@thinkyhead thinkyhead merged commit 7f8133a into MarlinFirmware:RCBugFix Nov 23, 2016
@Kaibob2 Kaibob2 deleted the CaseLightMenu branch November 23, 2016 20:08
@Kaibob2
Copy link
Contributor Author

Kaibob2 commented Nov 23, 2016

Puh, this was a tough one.
@esenapaj @thinkyhead Thanks a lot for your patience. I really learnt a lot about github today.
I already have another PR in mind, but i'm a little uncertain if it will become a disaster like this one...

@thinkyhead
Copy link
Member

thinkyhead commented Nov 24, 2016

@Kaibob2 Practice makes perfect! If you have a BASH command line installed on your system (or can add one), I've written a few helper scripts that I find invaluable for working on Marlin. See #3567

thinkyhead added a commit that referenced this pull request Nov 25, 2016
Follow-up the PR #5255 (Case light menu (3rd attempt))
@thinkyhead thinkyhead mentioned this pull request Nov 28, 2016
thinkyhead added a commit that referenced this pull request Dec 16, 2016
Fix for the PR #5255 (Case light menu (3rd attempt))
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.

3 participants