#84 Added support for Display (abstraction implementation) ...

[SrMouraSilva]

... PCD8544DisplayComponent (Nokia 3110 and 5110) and AWTDisplayComponent (for tests)

Hi, Im SrMouraSilva and I added a initial support for Display LED based in Java Display2D

Please, see the #84 for some comments and see pi4j-example/src/main/java/DisplayExample.java for a simle use

[SrMouraSilva]

@natdan

[natdan]

It seems that there's mismatch of java versions in this pull request. We still didn't move to Java8... You seem to be using Optional from 1.8. Would it be too much a hassle if you redesign interface not to use Optional?
Also, while you are in the code - could you, please, add '{' '}' in all blocks (especially if/else) - if nothing else but for me! :)

And last thing is just a question - something for dicussion: Command interface (and some implementations). Do they really belong to the 'top level' part of the Display interface? To me they have nothing to do with anything regarding display and should go in 'impl' package. What do you think?

Otherwise it is pretty much as I imagined it and I can see it would be quite easy to implement SSD1306 (and similar) over it. I can already see improvements in the API: maybe add something like 'update()'/'setAutoUpdate()' which can batch changes before committing them to the hardware. Those changes can go in a in memory buffer keeping rectangles of changed bits and then at update() it will compose what to be sent to the hardware. That way two 'setPoint()' methods in the same byte in display buffer will go as one transfer rather than two. Does this make any sense? Of course - that's for some of the following iterations/improvements... :)

[SrMouraSilva]

I will see it calmly all soon, for now I will focus on testing multiple displays in parallel.

(Perhaps implement something like http://techstroke.com/wp-content/uploads/2011/09/multiple-monitors-in-winodws-8.jpg)

It seems that there's mismatch of java versions in this pull request. We still didn't move to Java8... You seem to be using Optional from 1.8. Would it be too much a hassle if you redesign interface not to use Optional?

But since starting the discussion on the "Optional", there is the following line in Readme.md in the "develop" branch "Now requires Java runtime 8."

I use optional to avoid to make a mistake with a Null (PointerException) ...

Initially see some solutions:

• Simply not send AWT test implementation (and, consequently, future implementations may not enjoy "DisplayBuffer" Java8 to be used in the project)
• Send also another implementation of Optional for Java6
• Expect the project to upgrade to Java8 :sad:
• Dealing with Nulls (really would not like it)

Also, while you are in the code - could you, please, add '{' '}' in all blocks (especially if/else) - if nothing else but for me! :)

I did not realize this particular project. Will do.

What remains to be seen, I will see as soon as possible

Good afternoon

(Google Translate, tkx! hueuhehuebrbr)

[SrMouraSilva]

And last thing is just a question - something for dicussion: Command interface (and some implementations). Do they really belong to the 'top level' part of the Display interface? To me they have nothing to do with anything regarding display and should go in 'impl' package. What do you think?

The "Command" (and ByteCommand) is very generic and can be used in other components that transmit bits as commands. I put in "utils" for not knowing where to put it.

To me they have nothing to do with anything regarding display and should go in 'impl' package.

???
You are referencing these?

• DisplayBuffer.java
• PixelBuffer.java

If you are of them are talking about, I think are generic enough to be used in various Displays implementations

I could not understand, could try to explain in a different way?

• Do they really belong to the 'top level' part of the Display interface?
  Who?
• To me they have nothing to do with anything regarding display and should go in 'impl' package.
  So to you all that does not describe the "Display" or something related shall be a child package?

[SrMouraSilva]

I can already see improvements in the API: maybe add something like 'update()'/'setAutoUpdate()' which can batch changes before committing them to the hardware.

Your update() is my Display.redraw() / DisplayGraphics.dispose(). I believe dispose() should be extinguished in any way, as for the Java Graphics API dispose() is something else. Maybe, change "redraw()" method name for "update()"... But redraw() is more explicit

setAutoUpdate() is very cool but exists a problem in current implementation that create it now is possibly a bad idea:
The draw is syncronized

In current implementation (usign my GPIO software shiftOut [not the WiringPi software shiftOut]), don't work:

I'm trying use 2 displays PCD8544 in parallel mode*, but the concurrency in my specific application makes possibly the two displays change data simultaneously in the same pins o.O
Only one display prevail 😞

*Paralel mode: all the pins of all displays are in paralel. except the ChipEnabled. It's inspired in: http://wiringpi.com/dev-lib/lcd-library/ (It is possible to wire up more than one display...)

That's why, even using Display Graphics, I did be required to call dispose () to refresh the screen explicitly

[SrMouraSilva]

Those changes can go in a in memory buffer keeping rectangles of changed bits and then at update() it will compose what to be sent to the hardware. That way two 'setPoint()' methods in the same byte in display buffer will go as one transfer rather than two. Does this make any sense? Of course - that's for some of the following iterations/improvements... :)

If I really understood, you want to keep an area of rectangular to only update this area in order to save transfers.

Well, in my implementation, rather than upgrade a rectangle, it updates only pixels that were actually updated

That is why the existence of DisplayBuffer and PixelBuffer.java (for recording states of single pixels at a time). And PCB8544DDRamBank and PCB8544DisplayDataRam for PCD8544 (because in PCD8544 a byte represents 8 pixels).

Example:

display = new PCD8544Display(...)
graphics = new DisplayGraphics(display)

graphics.drawRectangle(Black in all pixels of display)
display.dispose() // Print black in all pixels
graphics.drawRectangle(White in (all pixels of display)/2)
display.dispose() // Print only the pixels changed (the white pixels)

But

display = new PCD8544Display(...)
graphics = new DisplayGraphics(display)

graphics.drawRectangle(Black in all pixels of display)
display.dispose() // Print black in all pixels in the real Display

graphics.clear() // Set all pixels to White
graphics.drawRectangle(Black in all pixels of display)
display.dispose() // Print black in all pixels 

// The correct would be that no change be sent because there was no submission to make a clear
// It's neccessary create a test and fix it

// Fixing it, my implementations will make the update in real Display will have complexity O(n), where n is the number of pixels actually changed

[natdan]

Sorry for confusion: Command interface and subsequent (abstract and otherwise) implementations are not 'display' specific. They shouldn't be in 'display' package. Even though I agree those being generic and can be used with any implementation of, pretty much anything, they do not belong to 'display' package. I am OK for Command interface and classes to live in 'display.impl' and other implementations to use it. It would be nice those being refactored even higher up to, for instance, com.pj4j.component.util.
Does that make sense?

'update' vs 'redraw': I can see you've already implemented change list in DisplayBuffer...

Java 8 wise - if we are moving to Java 8 then fine it makes sense... And you are right it is already in README.md.

[natdan]

"Well, in my implementation, rather than upgrade a rectangle, it updates only pixels that were actually updated" - yes, I am sorry I missed that! So you were already implementing it anyway. Great!

[SrMouraSilva]

• Fix {} in blocks
• Wait the java 8 or add a Optional implementation while is not updated (Version 2.0 Java 8 #203)
• Change the package location of not abstract display things -> com.pj4j.component.display.impl
• Moves the realy Component generics for com.pj4j.component.util
• Create test for buffer changes
• Fix unneccessary changes
• Change the DisplayGraphics to force use DisplayBuffer
• Rack their brains to figure out how to make changes in asynchronous display and not interfere with other displays using the same pins O.O Just use a pin for controller
• Force use display graphics? (to don't setPixels diretty in the display?)
• Pull request

@natdan, thanks for your attention and your opinions! :D

[savageautomate]

Is this change ready for a merge?

[SrMouraSilva]

Wait the java 8 or add a Optional implementation while is not updated

Hi! It's neccessary Java 8

Java 8 has marked with "Release 2.0" (#203)

[savageautomate]

I am planning on creating a new branch for Version 2.0 (requiring Java 8) soon. I'm just trying to get everything wrapped for for the 1.1 release.

[SrMouraSilva]

Then I will wait the branch. All ok? :)

(This merge request has not been updated with last implementations)

Good morning

[savageautomate]

OK, sounds good!

[savageautomate]

Im going to close this pull request for the time being. We can open a new pull request when we upgrade to Java 8 for version 2.0.

[SrMouraSilva]

Ok!

[SrMouraSilva]

Any news?