-
-
Notifications
You must be signed in to change notification settings - Fork 19.2k
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
Feature request: add suppport for reprapworld keypad #1142
Comments
i just used this line and the full features work fine #define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER On Wed, Nov 12, 2014 at 10:44 AM, vanhooren notifications@github.com
|
OOPs copy pasted the wrong line. I edited the original post , the issue still stands though |
i think i misunderstood your post about the keypad I'm not sure how you use both the rotary dial and that together On Wed, Nov 12, 2014 at 12:10 PM, vanhooren notifications@github.com
|
did you get the issue solved? |
not yet |
but the issue is still there even if you take a fresh copy of marlin? |
Just downloaded the latest marlin version, set board to 33 (Ramps1.3-1.4)
and
these are the errors i get when trying to compile
|
ucommenting #define REPRAP_DISCOUNT_SMART_CONTROLLER and so do the combinations Keypad + #define RA_CONTROL_PANEL the keypad does not play nice with its fellow accesories I do wish to thank you for devoting precious time to this problem |
Just notes:
|
Stepping through the code, the line referenced is 1371. WRITE(SHIFT_CLK,LOW); Tons of preprocessor directives here, which ends up translating SHIFT_CLK to DIOSHIFT_CLK_PIN and DIOSHIFT_CLK_WPORT, which aren't defined. As for BLEN_REPRAPWORLD_KEYPAD_F3 and related, there is a definition, but it's in a preprocessor elif. It looks like because REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER was defined in Configuration.h, the automatic expansion is pulling DOGLCD in there too, and the elifs ran by the preprocessor are expanding that one and not the keypad's. But the keypad is still defined, so the lcd update code uses it. Removing the doglcd define from line 584 of Configuration.h clears the symptoms for that particular compile error, but I'm wondering if there's any misconfiguration here as well. @vanhooren, do you have both the keypad and the full graphic controller? If you don't (and instead have a slightly different LCD), that may be your problem. |
Yeah, it's all caused by defines. I think when you define both the controller and the keypad, the keypad defines get used in one part but not in another. This could be fixed by either having one completely override the other or merging the functions that the two define so there isn't a problem. Another option would be to add more configuration so you could disable the input functions of the graphic controller. What a mess -.-; Is having both of these items at the same time a common use case? Both compile in isolation, so I don't think there's any missing functionality. |
I don't think having an lcd and the keypad is that common but neither is it ever going to be I do have the full graphics controller and not a similar lcd. Again thanks for all the valuable time put into this. |
I think that entire area needs some refactoring to get some flexibility out of it. I can work on it, but it's going to take some time. Additionally, I don't have any of the HW so it's going to be "fun" to breadboard up a mock to test this -.-; I'm also investigating adding tests that can be kept with the source. No sense breaking things unknowingly if we don't have to. |
you could always ask our testers to check your fork of marlin.. there are a few with a display.. also remember that each of these devices need their own CS line to the MCU... or they will clash when talking to the MCU.... the MCU pulls this CS line high when it wants to talk to a device... the only common are the data lines... even if there is only one device it have an individual CS line |
Thanks for the reminder. I tend to do a lot of short, iterative runs, so a really long feedback loop will probably produce poorer code. My thought process at this point is to allow for two "slots" for controls, one for a LCD/button and another for just controls. If you're using more than one LCD at the same time, you should probably be writing your own support for it (as such as user knows more about what they want to do with multiple LCDs than we :D). |
The reprapworld keypad is something that we do not support. If you have a reprapworld keypad, Ask reprapworld for support. |
I was mostly thinking about refactoring that area so that there is an Also who did write the support for the keypad?
|
Do we want to pull the existing support out of firmware until it is
|
Unfortunate reprapworld keypad users. They start working on it and stopped. And should not pull the existing support out. But we do not active maintain it. |
If we're not intending to do something about this, then we should close the issue and/or relabel it. At least take it out of the Bug Fixing milestone. So, "not a bug"? Just a configuration that is currently known not to work? |
To be fair I do believe the keypad works with the Reprapworld hardware (LCD, mini or megatronics and the keypad). It would be nice if this threat stayed visible so future prospective buyers are aware of this. Again I would like to thank all of you who have devoted valuable time to this. |
me and @ErikZalm had a brief talk about this one on IRC chat.... reprapworld should provide support for their hardware not us.... users are welcome to add the support... but marlin as a project should not do it, unless we are total out of issues and PR's and are so bored... but that is not going to happen for a long time... so the conclusion is to rename this as a feature request and remove it from the milestone... |
Kooky idea: Can a single CS line be doubled, such that when it goes low/high it causes a second line to go to the opposite state? Could such a hardware solution address SPI conflicts like this? |
@thinkyhead You'd need an inverter in front of one of the SPI CS pins. This has the side effect of not being able to turn off a device. Unless you're flat out of pins, it's better to give each CS its own pin. If you had a lot of SPI devices, you could MUX the CS line, though--you'd need log2(N) pins to do that though, where N is the number of SPI devices. You'd increase the latency for any specific device, though, if you wanted to keep polling everything. For this keypad, the other problem is that only 1 input and 1 output peripheral can be installed at the same time. This is because the source defines structures and functions, and C gets unhappy if you have 2 structs defined that have the same name, or 2 functions with the same signature. |
i will close this one.... support for new hardware should still come from the inventor and not us... |
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. |
I can't seem to get the marlin firmware to compile when the following is selected
motherboard 33 ''ramps"
the firmware will compile when
Selecting both these will not compile in any marlin version i tested.
I understand that there is some overlap in functionality between the reprap discount rotary encoder and the keypad rotary encoder perhaps this is the problem?
Unfortunately I am not a programmer and between learning 3d modeling and learning about printing in general i doubt i will have time to delve into it in the next couple of weeks.
I will do my best to understand the issues further.
I was planning on doing vb.net evening courses but that is still a couple of months away.
When trying to compile with board set to 33 (ramps 1.4)
I get the following error log
when compiling with only the
#define REPRAPWORLD_KEYPAD
uncommentedi get the this error
compiling with only the full graphics controller has always worked fine
If i missed something bleedingly obvious i am sorry for wasting time
I have found issues that debate a similar problem but found no solution that could be applied to my
exact situation
The text was updated successfully, but these errors were encountered: