selfcustody / krux

Open-source signing device firmware for Bitcoin
https://selfcustody.github.io/krux/
Other
179 stars 36 forks source link

Different displays and cameras #147

Closed odudex closed 1 year ago

odudex commented 2 years ago

As more people run Krux, more hardware variations show up, not only on custom kits like Bit and Dock, but also Maix Amigo. I'll just point here this demand. Down the road we can add mirror and rotate configuration options for camera and display so users can solve compatibility issues without the need of custom firmware.

ghost commented 2 years ago

Oof. Yeah, letting the user manually adjust mirror and rotation in their settings sounds better than having to build custom firmware from source. The only downside there is they would need to use a microSD to persist the changes (since the internal filesystem is unfortunately read-only due to using memzip).

As for this particular example, do you know what display is being used in the mirrored Amigo? Is it something we can support?

odudex commented 2 years ago

There is another issue. We've been calling in code the working version IPS, but, thanks to @kkdao, I found out most of devices are TFT, including mine, and probably yours. In the case from picture above, I thought one was TFT, other IPS, but user said both are labeled as TFT. He will check if he can find any clue about the difference. Here is where it says it is TFT where the label is

kkdao commented 2 years ago

The first thing you said is true, but the second part was a dev on Telegram who has been playing with these boards and firmware as well: https://t.me/DIYbitcoin/4537

ghost commented 2 years ago

There is another issue. We've been calling in code the working version IPS, but, thanks to @kkdao, I found out most of devices are TFT, including mine, and probably yours. In the case from picture above, I thought one was TFT, other IPS, but user said both are labeled as TFT. He will check if he can find any clue about the difference. Here is where it says it is TFT where the label is

Both devices are running the same firmware / git commit?

odudex commented 2 years ago

Both devices are running the same firmware / git commit?

Yes, AFAIK

ghost commented 2 years ago

It does look like this may be due to some of the devices being IPS and some TFT, even if the box itself says it is TFT, per: https://github.com/sipeed/MaixPy/issues/443#issuecomment-976205809

Further, looking at the default configs in this repo: https://github.com/sipeed/MaixPy_scripts/tree/master/board

You'll see that https://github.com/sipeed/MaixPy_scripts/blob/master/board/config_maix_amigo.py is not inverted while https://github.com/sipeed/MaixPy_scripts/blob/master/board/config_maix_amigo_ips.py is inverted. This leads me to believe that both of our devices are IPS while the user's in this issue is a TFT.

To resolve this, I'll update the configs of IPS and TFT to match the values found in this repo and update the build process to include builds for both types. Users will just have to try each of them on their machine to see which works.

odudex commented 2 years ago

One thing to note is that the characters are not mirrored on the supposed TFT, only their x coordinates. In Amigo, if you don't set the "mirrored" flag, characters and words will be displayed mirrored, so I'm afraid setting this off on TFT will fix the x coordinate issue, but will flip characters, but worth trying.

ghost commented 2 years ago

I think this should do what we want? https://github.com/selfcustody/krux/pull/148/commits/09852c77ce5b687044f3b519bdbbc4eb84242d15

The character drawing code remains as-is, checking for the invert property which will be either 1 or 0 depending on which amigo is present

odudex commented 2 years ago

That's a good idea. Now take a look a this. Maybe this is the IPS? amigo_screen_ips

odudex commented 2 years ago

Managed to make this 3rd type work by using this config:

lcd.init(invert=True) #this invert is about colors
lcd.bgr_to_rgb(True)
lcd.mirror(True)

PS: The x coordinates on this are OK with standard code

ghost commented 2 years ago

Wow! There really are a lot of different displays... 😰

If this is the IPS, then perhaps the invert key in the config was referring to color inversion all along.

odudex commented 2 years ago

perhaps the invert key in the config was referring to color inversion all along

I think so. If there were someway the LCD could return its type or model we could solve this in a neat way. For cameras I'm thinking modify the settings according to sensor.get_id() instead of using configs, that would clean the configs and be more flexible in cases like the Bit, which they ship it with different camera models.

odudex commented 2 years ago

Managed to make cameras work without looking at board.py configs. It configs using sensor ID as reference:

sensor.reset()
        self.ID = sensor.get_id()
        sensor.set_pixformat(sensor.RGB565)
        sensor.set_framesize(sensor.QVGA)
        if self.ID == OV5642_ID:
            sensor.set_hmirror(1)
        if self.ID == OV2640_ID:
            sensor.set_vflip(1)
...

if self.ID in (OV2640_ID, OV5642_ID):
                img.lens_corr(1.2)

sensor can be removed from board.py configs In the process, I also made all 4 devices use the same parameter for"orientation":[1, 2]. So this could also be removed from board.py

ghost commented 1 year ago

@odudex I've forgotten where we left off on this. Is this an issue we can close out now, or should I leave it open?

odudex commented 1 year ago

There's one undone cleanup task: Remove "sensor" attribute from board.py of each project on Maixpy as it is not used anymore.