JohnDoneth / hd44780-driver

Implementation of the embedded-hal traits for the HD44780.
MIT License
37 stars 40 forks source link

Fix #38 #40

Closed chevdor closed 2 years ago

chevdor commented 2 years ago

As pointed out by the author of #38 the current implementation is wrong. This PR addresses the issue and also brings a little more comfort to the user thanks to the new set_cursor_xy function.

This first photos show the result on a 2004 LCD writting at position 1, 11, 21, 31. While 1 and 11 land in the right spot, we see that the rest is off.

PXL_20220629_093931053

This PR fixes the issue as we can see below: PXL_20220629_105054487

but also lets the user use the more convenient set_cursor_xy function.

chevdor commented 2 years ago

Yes unfortunately, my editor did fmt quicker than me :) Sorry for the noise. The changes are rather small though. I was not sure if this repo is maintained but happy to help with adding fmt and clippy to the CI to avoid that in the future.

JohnDoneth commented 2 years ago

No worries. Adding those to CI would be awesome. Looks like CI is currently bugging out on the embassy feature. This project is quite old but I try to keep it somewhat maintained although some PRs slip through on accident. Is this good to merge? Does it compile locally for you? I see it's just embassy / async

JohnDoneth commented 2 years ago

Thanks for this! ❤️