Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use generics for rows and columns #3

Merged
merged 4 commits into from
Oct 10, 2024
Merged

Conversation

JomerDev
Copy link
Contributor

@JomerDev JomerDev commented Oct 1, 2024

This PR removes the rows setting and instead uses generics for rows and columns.
This allows for asserts in some places to make sure the users aren't passing in values larger than the set rows or columns values.
With this support for screens up to 20x4 was added (I know the name of the library specifies 1602 but I don't think there is a point in not supporting them, it only cost some very small changes)

(I'm also currently preparing a PR to support the creating of new characters which the LCDs allow as well)

Provide some default types
@KuabeM
Copy link
Owner

KuabeM commented Oct 2, 2024

Nice, thanks! I'll review it in the upcoming days :)

Would you mind adding a comment to the readme that other 'formats' are also supported? I assume you tested this with some hardware, so you could even add a link/reference to it.

Also, looking forward to the custom character PR!

@JomerDev
Copy link
Contributor Author

JomerDev commented Oct 4, 2024

Done. The formatting is also fixed

I'll open the custom character PR after this one is merged, I made the mistake of basing it on these changes

@KuabeM
Copy link
Owner

KuabeM commented Oct 7, 2024

Reviewed the code, looks good to me :) I don't have anything at hand to test it.

Where you able to test it with all the different supported sizes?

src/async_lcd.rs Outdated Show resolved Hide resolved
@JomerDev
Copy link
Contributor Author

JomerDev commented Oct 8, 2024

I tested it with a 20x4 LCD, the others I haven't checked. But the specific changes came again from the C++ library I mentioned before, so anything they support should work here now as well

@KuabeM
Copy link
Owner

KuabeM commented Oct 8, 2024

Actually, there is a simple way! You just need to wrap the assert! in a const block. They are supported since rust 1.79.0, see the rustc PR

        const { assert!(COLUMNS > 0, "COLUMNS needs to be larger than zero!") };

The error messages are not super nice, but better then a runtime fail.

Could you add these consts?

@JomerDev
Copy link
Contributor Author

JomerDev commented Oct 9, 2024

Done. I could have sworn I tried that, but I think I didn't add the brackets around the assert.
The asserts in set_cursor can't be const since the arguments to the function aren't const

@KuabeM KuabeM merged commit b8b7460 into KuabeM:master Oct 10, 2024
3 checks passed
@KuabeM
Copy link
Owner

KuabeM commented Oct 10, 2024

Thanks again, looking forward to the custom char PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants