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

Added support for SH1106 and SSD1306 LCD modules. #2574

Conversation

fmalpartida
Copy link
Contributor

Added support for SH1106, SSD1306 LCD drivers found in most OLED driver.
Added support for the SAV OLEd LCD module.

Added support for the SAV OLEd LCD module.
@AnHardt
Copy link
Member

AnHardt commented Aug 1, 2015

Did you try the X2 devices?

@fmalpartida
Copy link
Contributor Author

X2?

Enviado desde mi iPhone

El 1/8/2015, a las 14:39, AnHardt notifications@github.com escribió:

Did you try the X2 devices?


Reply to this email directly or view it on GitHub.

@Wackerbarth Wackerbarth added Needs: Work More work is needed C: Boards/Pins labels Aug 1, 2015
@fmalpartida
Copy link
Contributor Author

OK, I think it should be aligned to the ENABLED macro.
Currently going through travis

@AnHardt
Copy link
Member

AnHardt commented Aug 1, 2015

*_2x the devices with the doubled RAM-buffer. They need only 4 instead of 8 loops in the pictureloop.

See:
https://code.google.com/p/u8glib/wiki/device
and
https://code.google.com/p/u8glib/wiki/tpictureloop

That basically means the *_2X devices need only half the processing power. At least as long as our pictureloop is not changed. (That's my project for after the next release. I'm waiting a bit impatiently. :-) )

@fmalpartida
Copy link
Contributor Author

OK, ready to merge in.

@AnHardt , I haven't tried _2X LCDs. I don't think the ones I have are _2X but will surely try. I got them off eBay from a WangHooLow dealer advertised as SSD1306 and ended being SH1106.

@fmalpartida
Copy link
Contributor Author

Tested my lcds against 2X and are working nicely. Though I am not convinced that they are 2X.

Cheers,

fm

Enviado desde mi iPhone

El 1/8/2015, a las 18:29, AnHardt notifications@github.com escribió:

*_2x the devices with the doubled RAM-buffer. They need only 4 instead of 8 loops in the pictureloop.

See:
https://code.google.com/p/u8glib/wiki/device
and
https://code.google.com/p/u8glib/wiki/tpictureloop

That basically means the *_2X devices need only half the processing power. At least as long as our pictureloop is not changed. (That's my project for after the next release. I'm waiting a bit impatiently. :-) )


Reply to this email directly or view it on GitHub.

@thinkyhead
Copy link
Member

Looking good. I will give it a thorough look-over and if nothing offends my effete sensibilities I will merge it forthwith.

Hey, it occurs to me that the HAS_SOMETHING defines – although typically true or false could be plainly defined, but we would need to change #if HAS_SOMETHING to #if ENABLED(HAS_SOMETHING). But part of the idea of ENABLED / DISABLED is that they apply to configuration options that are switches, and not to things that are defined conditionally (except some things in Conditionals.h which is loosely a part of the configurations). So, as you were…

#if ENABLED(U8GLIB_ST7920) || ENABLED(U8GLIB_SSD1306) || ENABLED(U8GLIB_SH1106)
#define HAS_LCD_CONTRAST false
#else
#define HAS_LCD_CONTRAST true
#endif
Copy link
Member

Choose a reason for hiding this comment

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

The way most HAS_SOMETHING defines work would also work well here:

#define HAS_LCD_CONTRAST (ENABLED(DOGLCD) && DISABLED(U8GLIB_ST7920) && DISABLED(U8GLIB_SSD1306) && DISABLED(U8GLIB_SH1106))

I see HAS_LCD_CONTRAST is used in few other files, so those should be patched up to use #if ... too.

@fmalpartida
Copy link
Contributor Author

Why was it closed with no Merge?

@Wackerbarth
Copy link
Contributor

Frankly, I don't know. I didn't even realize that I did so. (I must have clicked in the wrong window).

However, it DOES need to be closed and resubmitted in MarlinDev since it is something new for 1.1 and not just a correction to the 1.0 branch.

@fmalpartida
Copy link
Contributor Author

OK, I thought it was picked up just before creating version 1.1 and was automatically pulled into 1.1.

Is there an easy way to pull it to MarlinDev?

@Wackerbarth
Copy link
Contributor

Considering the nature of the change, I would make a copy of the three files somewhere conveniently located on your host. Then I would checkout the current version of "master" and replace the files in question.
then you can use git diff, etc. to merge in your changes while picking up other changes that have occurred in Configuration.h, etc. Then, submit a pull request on MarlinDev.

While you are at it, you will need to make the same additions to ALL of the example configurations.

(Or join the mutiny to split up Configuration.h so that we no longer have to do it that way)

@fmalpartida
Copy link
Contributor Author

What mutiny? It is a real pain to have to update all the example configuration files out there. This is what I was referring to... If people want to keep their example configurations up and running they should maintain them. Most of them are the same thing for their own machines.

@Wackerbarth
Copy link
Contributor

The pain is because someone adds some option which does not apply to "my" machine. Yet Scott insists that my Configuration.h file be modified to include that option, commented out, in case some user wants to add it to his configuration which he based off of mine.

As a maintainer of a configuration, I should not be expected to do anything to add every new option, especially when they do not apply to my machines.

As an example, consider the LCD displays. In general, they are a separate component. Rather than having a copy of the LCD configuration in EVERY Configuration.h, there could be just one (somewhat like Configuration_adv.h). Then, when you add a new LCD option, it doesn't require everyone to update their Configuration.

@fmalpartida
Copy link
Contributor Author

OK, PR generated on MarlinDev, didn’t take much to publish it there. I’ve tested it and it all works as expected.
Can you see the PR? I think is 100.

Cheers,

fm

On 20 Aug 2015, at 18:36, Richard Wackerbarth notifications@github.com wrote:

The pain is because someone adds some option which does not apply to "my" machine. Yet Scott insists that my Configuration.h file be modified to include that option, commented out, in case some user wants to add it to his configuration which he based off of mine.

As a maintainer of a configuration, I should not be expected to do anything to add every new option, especially when they do not apply to my machines.

As an example, consider the LCD displays. In general, they are a separate component. Rather than having a copy of the LCD configuration in EVERY Configuration.h, there could be just one (somewhat like Configuration_adv.h). Then, when you add a new LCD option, it doesn't require everyone to update their Configuration.


Reply to this email directly or view it on GitHub #2574 (comment).

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

Successfully merging this pull request may close these issues.

None yet

4 participants