peterhinch / micropython-font-to-py

A Python 3 utility to convert fonts to Python source capable of being frozen as bytecode
MIT License
368 stars 67 forks source link

Use ducktyping instead of type checking // Using multiple framebuffers #29

Closed taxus13 closed 4 years ago

taxus13 commented 4 years ago

Hi, thanks a lot for your great work. I managed to get it to work on a Waveshare 4.2 two color e-paper display. See also #25. The driver itself uses two framebuffers, one for storing the black part of the image, one for the red part: https://github.com/mcauser/micropython-waveshare-epaper/blob/master/epaper4in2b.py

Due to this, I implemented a Driver class with the corresponding methods the driver uses and an additional switch to select the buffer the writer should write to: https://gist.github.com/taxus13/a4fe57519faf7d78303fe1cb0bae4fe4

To make the Writer work with my driver I had to modify it slightly:

What do you think bout the type chening? I think, explicit type checking is not necessary, we can just use duck typing. What do you think about the additional parameter for color selection?

peterhinch commented 4 years ago

Why did you use two frame buffers rather than the CWriter class as used here - see pictures?

taxus13 commented 4 years ago

Because the driver explicitly uses two framebuffers: https://github.com/mcauser/micropython-waveshare-epaper/blob/master/epaper4in2b.py#L125 First, the framebuffer containing black is transferred, then the red framebuffer is transferred. Both of them are strictly monochrome.

peterhinch commented 4 years ago

Sorry if I'm being slow here, I hadn't cottoned on that the display driver was written to use two frame buffers. However the Writer class already supports multiple frame buffers via its device arg. This is intended to support multiple displays (each with its own framebuf). Your case is two logical displays mapped onto one physical unit.

In which case why not use multiple Writer instances and treat the red and black displays as separate devices? You already have an instance for each font. It seems logical to have an instance for (say) red_arial and another for black_arial.

taxus13 commented 4 years ago

Yes, that would be working, but I would have to synchronize the text position manually. With my implementation, I can simply switch between the colours, which is very neat.

peterhinch commented 4 years ago

OK, I see the benefits. For the moment I think you'll need to maintain your own fork - putting these changes into my code would require me to buy an identical display in order to test. I'm very busy dealing with the new uasyncio, but I'll look at this again when I'm done.

The application is rather specialised, being limited to users of that display who don't want to treat it as two devices.

taxus13 commented 4 years ago

Thats fine, thank you!

One step in that direction would be to remove the isinstance checking - why do you check for the type there?

peterhinch commented 4 years ago

The purpose of the Writer class is to interface to display drivers which are subclassed from framebuf. With other display drivers it would fail at runtime by trying to call nonexistent methods (e.g. .blit()). It seems prudent to trap this user error at the earliest opportunity, with an unambiguous message.

taxus13 commented 4 years ago

Ah I see. You could replace that by explicitly checking if the methods are existing: https://stackoverflow.com/a/5268474 Could that be an alternative?

peterhinch commented 4 years ago

Yes.

I'm puzzled what you want me to do. As I explained I have no plans at the moment to support dual framebuf drivers; I suggested you maintain your own fork. By all means use getattr in your fork.

The Writer test is deliberately more specific. It requires the device to be subclassed from framebuf because the class uses multiple bound variables and methods of the base class. Rather than multiple getattr calls to check everything it uses from the base class it's far simpler to check for inheritance.

The design aim for Writer is to support only devices derived from framebuf. The code checks for this in the simplest way. I see no benefit in changing this test.