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

Ulticontroller click encoder rotation direction is reversed #3

Open
RedwoodForest opened this issue Sep 5, 2013 · 10 comments
Open
Assignees

Comments

@RedwoodForest
Copy link

With Marlin built from Daid's builder to increase print speed I would turn the encoder clockwise. With Marlin from this builder I have to turn it counter-clockwise. The same direction swap applies to menu navigation and everything else I've tried. The old way seemed much more intuitive to me.

https://groups.google.com/forum/#!topic/ultimaker/eg2Td3DcF7s

@ginge
Copy link
Owner

ginge commented Sep 5, 2013

OK Now I am confused.

If I define ULTIMAKERCONTROLLER only, everything works but the knob is inverted.
If I define ULTIPANEL only, then it doesn't work at all.

Daid's default configuration.h:

#define ULTIMAKERCONTROLLER //as available from the ultimaker online store.
//#define ULTIPANEL //the ultipanel as on thingiverse

clearly the same config option enabled!

There is a significant difference between the LCD code in Ultimaker/Marlin or daid/Marlin and ErikZalm/Marlin branches.
Changes were merged in ErikZalm 3 months ago: ErikZalm/Marlin@6a81291

At this point I am going to go with it's a bug in ErikZalm's branch. The daid/ultimaker branch is a few months behind and uses older code. The dead giveaway is ErikZalms latest patch that changes the pitch of the buzzer to be less annoying (Although I do miss the old chirpy noise)

Can someone check my logic before I send a patch

@ginge
Copy link
Owner

ginge commented Sep 5, 2013

I have created a pull request in ErikZalm's branch.

https://github.com/ErikZalm/Marlin/pull/587

The previous patch has been modified to this:

  • #ifndef ULTIMAKERCONTROLLER
  • #ifndef ULTIMAKERCONTROLLER_DEV

if anyone has an oldstyle LCD, just define ULTIMAKERCONTROLLER_DEV in Configuration.h
Support for the limited release -dev ulticontrollers will likely not be added to the builder page.

After testing this works for me.

@RedwoodForest
Copy link
Author

I just tested and the current version works for me too. I'll let you close the issue once you've resolved the LCD code discussion with Daid.

Thanks!

@gr5
Copy link

gr5 commented Sep 10, 2013

This is now fixed in the latest build. I had also created a pull request (not realizing ginge created one too). Erik pulled my merge. I tested the fix and it now works on my ulticontroller (was backwards without the fix). Please close this issue and refer to pull request #594.

@ginge
Copy link
Owner

ginge commented Sep 10, 2013

Confirmed as working with the patch from #594.

I will await lawrencejohnston's input before closing.

Good work all round!

@ghost ghost assigned ginge Sep 10, 2013
@RedwoodForest
Copy link
Author

I have no problem closing it based on your testing. Thanks to both of
you for fixing the issue.

@gr5
Copy link

gr5 commented Sep 11, 2013

The saga goes on – just spent 3 hours looking this over again. Lol. But I stand by the original pull request...

https://github.com/ErikZalm/Marlin/pull/594

  • George

@ginge
Copy link
Owner

ginge commented Sep 11, 2013

Agreed. I also just spent 2.5 hours sanity checking Marlin code, your commit and finally verifying with 3 machines I have access to. I stand by your changes.

Thumbs up here.

@gr5
Copy link

gr5 commented Sep 11, 2013

The issue isn't "does it work on ulticontroller". The issue is did I break something for other controllers.

The same code is used for 4 or more different types of controllers.

@ginge
Copy link
Owner

ginge commented Sep 11, 2013

To clarify, the code change works for UltiController and a Paneloulu. The latter using NEWPANEL encoder support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants