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

ECC code mismatch detection #526

Merged
merged 5 commits into from
May 19, 2024
Merged

Conversation

Shane32
Copy link
Contributor

@Shane32 Shane32 commented May 18, 2024

This pull request enhances the QRCoder library by allowing the payload to specify an "unspecified" error correction level (ECC) rather than being restricted to the predefined levels L, M, Q, or H. This new Default value in the ECCLevel enum acts as an unspecified placeholder, giving users the flexibility to select an appropriate ECC level while enabling the system to validate the selection.

When generating a QR code, if the ECC level is not specified, the system uses the payload's specified ECC level. If the payload's ECC level differs from the user-specified level, the system throws an exception to handle the discrepancy. This enhancement ensures backward compatibility, allowing existing codebases to function as expected while offering users the flexibility to dynamically adjust error correction levels based on their payloads' needs.

See:

This PR also includes bug fix #527 ; please merge it first.

@codebude
Copy link
Owner

That's a nice solution. I wonder if we shouldn't just include it in the next 1.x.x release. I don't see any reason why we need to keep the change for version 2.

Just a small test case (something like generator_throws_argument_exception or similar) would be nice before merging it.

@Shane32
Copy link
Contributor Author

Shane32 commented May 19, 2024

This particular implementation is a breaking change since I’ve changed the type. But we could add an enum value like EccLevel.Default to do the same thing, which would be backwards compatible. I’ll revise.

@Shane32 Shane32 force-pushed the ecc_mode_mismatch branch from 03174ac to 4ca3ec9 Compare May 19, 2024 13:56
@Shane32 Shane32 changed the title [Demo] ECC code mismatch detection ECC code mismatch detection May 19, 2024
QRCoder/PayloadGenerator.cs Outdated Show resolved Hide resolved
@codebude codebude merged commit 1030701 into codebude:master May 19, 2024
3 checks passed
@csturm83
Copy link
Contributor

Isn't it a breaking change to throw an exception when previously didn't? Might be worth mentioning in the release notes.

@codebude
Copy link
Owner

@csturm83 Thanks for pointing this out. Yes, you're right - this is somehow breaking behaviour but In my opinion nevertheless totally worth it, as cases when the exception will be raised, otherwise would lead to incorrect payloads/QR codes. (Thus I assume the change won't break nearly none implementation out in the wild, because to break it, the implementations must create in correct codes at the moment.)

I added a note on this change in the release notes:

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.

3 participants