robweber / omni-epd

An EPD (electronic paper display) class abstraction to simplify communications across multiple display types.
GNU General Public License v3.0
81 stars 17 forks source link

Auto inky #60

Closed donbing closed 2 years ago

donbing commented 2 years ago

This adds support for the inky autodetect from eeprom feature.

This has been tested against a red inky 4.2",Impression 5.6" and impression 4.0"

robweber commented 2 years ago

Is the Auto mode available for both pHat and wHat boards? Looking at the Inky library I think it is. You could include that in the README as well. Guessing this will also work for the 5.7in 7-color display? I don't that was available when these were first added.

Also, I tried to run this on my Inky pHat and got a "No EEPROM detected" error. Are there additional steps or libraries needed for this to work?

donbing commented 2 years ago

Some much older models did not have a compatible eeprom. It's either that, or i2c being disabled I'd guess

robweber commented 2 years ago

I'll try this out on my Inky tonight. I have a feeling the I2C might be the reason I couldn't get it going earlier.

If this will work for the Impression as well we may be able to consolidate some code into a single Inky class at some point. It looks like there is a fair amount of overlap, just need to code around the differences by checking the device type. That can be refactored at a later date though. Most people will probably end up using inky.auto once this is available anyway.

missionfloyd commented 2 years ago

Yeah, that's what I was thinking. InkyDisplay already handles multiple display types, it might as well handle the impression, too.

class InkyDisplay(VirtualEPD):
    """
    This is an abstraction for Pimoroni Inky pHat and wHat devices
    https://github.com/pimoroni/inky
    """

    pkg_name = INKY_PKG
    mode = "black"  # default mode is black, unless inky.impression
    modes_available = ("black")

    deviceList = ["phat_black", "phat_red", "phat_yellow",
                    "phat1608_black", "phat1608_red", "phat1608_yellow",
                    "what_black", "what_red", "what_yellow", "auto", "impression"]

    def __init__(self, deviceName, config):
        super().__init__(deviceName, config)

        # need to figure out what type of device we have
        dType, dColor, *_ = deviceName.split('_') + [None]

        if(dType == 'phat'):
            deviceObj = self.load_display_driver(self.pkg_name, 'phat')
            self._device = deviceObj.InkyPHAT(dColor)
        elif(dType == 'phat1608'):
            deviceObj = self.load_display_driver(self.pkg_name, 'phat')
            self._device = deviceObj.InkyPHAT_SSD1608(dColor)
        elif(dType == 'what'):
            deviceObj = self.load_display_driver(self.pkg_name, 'what')
            self._device = deviceObj.InkyWHAT(dColor)
        elif(dType == 'impression'):
            deviceObj = self.load_display_driver(self.pkg_name, 'inky_uc8159')
            self._device = deviceObj.Inky()
            dColor = 'color'
        elif(dType == 'auto'):
            deviceObj = self.load_display_driver(self.pkg_name, 'auto')
            self._device = deviceObj.auto()
            dColor = 'color' if self._device.colour == 'multi' else self._device.colour

        if(self._device.colour == 'multi'):
            self.clear_color = self._device.CLEAN
        else:
            self.clear_color = self._device.WHITE

        # set mode to black + any other color supported
        if(self.mode != "black"):
            self.modes_available = ('black', dColor)

        # phat and what devices expect colors in the order white, black, other, 
        if(self.mode == "red" and dColor == "red"):
            self.palette_filter.append([255, 0, 0])
            self.max_colors = 3
        elif(self.mode == "yellow" and dColor == "yellow"):
            self.palette_filter.append([255, 255, 0])
            self.max_colors = 3
        elif(self.mode == "color" and dColor == "color"):
            from inky.inky_uc8159 import DESATURATED_PALETTE
            self.palette_filter = DESATURATED_PALETTE
            self.max_colors = 8

        # set the width and height
        self.width = self._device.width
        self.height = self._device.height

    @staticmethod
    def get_supported_devices():
        return [] if not check_module_installed(INKY_PKG) else [f"{INKY_PKG}.{n}" for n in InkyDisplay.deviceList]

    # set the image and display
    def _display(self, image):
        # set border
        self._device.set_border(getattr(self._device, self._get_device_option('border', '').upper(), self._device.border_colour))

        # apply any needed conversions to this image based on the mode        
        if(self.mode == 'color'):
            saturation = self._getfloat_device_option('saturation', .5)  # .5 is default from Inky lib
            self._device.set_image(image.convert("RGB"), saturation=saturation)
        else:
            image = self._filterImage(image)
            self._device.set_image(image.convert("RGB"))

        self._device.show()

    def clear(self):
        for _ in range(2):
            for y in range(self.height - 1):
                for x in range(self.width - 1):
                    self._device.set_pixel(x, y, self.clear_color)

        self._device.show()

This seems to work right with the inky impression with both inky.impression and inky.auto, black and color modes.

donbing commented 2 years ago

@missionfloyd I think your code will cause the impression to default to black mode, whereas it currently defaults to color.

I really think that all the displays should be defaulting to their most colourful mode, but that may break apps that depend on this lib :(

I've already done this code merge, pls check my PR into this PR

robweber commented 2 years ago

I tested this and the IC2 stuff was the problem on my end. It works great now.

My initial reaction was to say let's merge this in and then handle the merging of the Impression code; however I've rethought that the more I look at the code. If auto is going to work properly it needs to work for the Impression 100% too. The issues above with the colors and palette not being set right would all be solved by merging stuff together.

@donbing - I didn't look through your other PR entirely. Could you review @missionfloyd's info and what you've done and try to come up with a solid PR that has it all? Sorry if that's more work than you initially wanted to put into this.

missionfloyd commented 2 years ago

I think the empty InkyImpressionDisplay class can be removed (and the import in displayfactory.py)

And a tip: run stuff through flake8 before committing it, save yourself the hassle.

donbing commented 2 years ago

I think the empty InkyImpressionDisplay class can be removed (and the import in displayfactory.py)

And a tip: run stuff through flake8 before committing it, save yourself the hassle.

thanks, done.

robweber commented 2 years ago

Awesome - thanks for putting it together. I'll give a test on my end as well.

With the unit test you created, I see it's excluded from the test suite with the mark_skip decorator you added. Do you just run it manually as part of your process? I was thinking recently having some better testing to avoid one fix breaking another would be useful and having specific tests for specific displays would be helpful.

donbing commented 2 years ago

Awesome - thanks for putting it together. I'll give a test on my end as well.

With the unit test you created, I see it's excluded from the test suite with the mark_skip decorator you added. Do you just run it manually as part of your process? I was thinking recently having some better testing to avoid one fix breaking another would be useful and having specific tests for specific displays would be helpful.

I've been removing the skipped attribute to run tests on my Inky's here, yep. I cant see a good way to say 'don't run this test unless someone explicitly executes it'

I've beefed up the test to allow more thorough testing.. I'd have liked to add some proper unit tests, but the inky mock lib requires tkinter so I gave up on that for now..

having said all that, i've broken the tests atm :) will fix

donbing commented 2 years ago

I think this is now good-to-go, i've identified a few small cleanups and a bug still however. @robweber let me know if you'd like me to apply/fix these as part of this PR.

all fixed up. this should be good to go now.

robweber commented 2 years ago

Great, thanks for putting it all together. It is very satisfying seeing everything pulled into one display class now. I went to test it and have some issue with my RPI so I'm re-formatting the SD card now. I'll confirm again from a fresh build and then we'll get this thing merged.

missionfloyd commented 2 years ago

Checked inky.impression and inky.auto, color and bw modes, display() and clear(). Everything seems to work right.

robweber commented 2 years ago

I made a comment regarding an error I'm seeing with my pHat device. I also found a weird edge case that causes an "invalid mode" issue.

If you set a mode in the omni-epd.ini file and are using inky.auto as the device type the variable dColor is set to None. This cases the modes available to be ('bw', None). If in any mode other that black and white you get an invalid mode error. I image even if you got past this error other things wouldn't work right as the palette won't be set properly since the color conditional won't trigger. Could the device property device.colour be used to properly return the right color as part of the load_device function?

I'm happy to make these minor changes after merge if you don't want to keep messing with updating this PR. The guts are all there and working properly.

donbing commented 2 years ago

branch updated again, currently failing tests due to ERROR: Invalid requirement: '[dev]' (from line 6 of src/omni_epd.egg-info/requires.txt)

robweber commented 2 years ago

You caught me messing with the unit tests. I updated the setup.cfg file but hadn't yet fixed the automated test syntax. It's fixed in mainline now. I know this will run through so not worried about it.

Everything is working on my device now. I can use inky.auto and set the color mode if I want and everything displays properly.

donbing commented 2 years ago

You caught me messing with the unit tests. I updated the setup.cfg file but hadn't yet fixed the automated test syntax. It's fixed in mainline now. I know this will run through so not worried about it.

Everything is working on my device now. I can use inky.auto and set the color mode if I want and everything displays properly.

great sutff. yep it's working well on all my inky's so far too

robweber commented 2 years ago

I think it's probably good right?

Is that a Red wHat in the pic? Can I mark that as "tested" too?

donbing commented 2 years ago

I think it's probably good right?

Is that a Red wHat in the pic? Can I mark that as "tested" too?

yep, red what and impression 4.0 in those pics. I also have a yellow what, but it doesn't really seem worth testing on.

I think this PR is all good now. I did a few more tests mixing in rotation and sharpness, all seems fine.

missionfloyd commented 2 years ago

Looks good to me

donbing commented 2 years ago

Thanks for all your help guys 🤩.

I'm keen to add some automated testing to InkyDisplay but that'd require extracting load_device() somewhere more easily mocked, so I'll leave that for another branch.

if you ever need any testing doing, give me a shout!