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

added SlovenianUpnQr payload generator, manual QR version and ECI mode #111

Merged
merged 3 commits into from
Apr 21, 2018
Merged

Conversation

ahotko
Copy link

@ahotko ahotko commented Apr 9, 2018

This PR implements the following features:

  • Added ECI Mode Support (partial)
  • Added possibility of Manual QR Code Version
  • Added Slovenian UPN-QR Code standard Payload Generator (SlovenianUpnQr)

ahotko added 2 commits April 9, 2018 13:18
…base class

Added Version, ECC and ECI to payload, so QRCode can be generated directly from Payload (ECC, ECI and version from Payload)
Added Overloaded QRCodeGenerator.CreateQrCode() methods that take Payload class as input
@codebude
Copy link
Owner

Hi @ahotko ,

thanks for your effort. Please give me a day or two to review the code and think about it. (I want to think about all possible side effects and conflicts. But from a first view, I think I'll merge it. 👍 )

Another question: Would you mind, to deliver a text for the wiki, which explains the SlovenianQRCode? Similiar to the SwissQrCode part over here: https://github.com/codebude/QRCoder/wiki/Advanced-usage---Payload-generators

@ahotko
Copy link
Author

ahotko commented Apr 10, 2018

Hei,

I tried to change the code as little as possible so the functionality SHOULD be compatible or the same.

SlovenianUpnQr generates string according to Slovenian UPN QR standard (UPN is acronym for "Univerzalni placilni nalog" or in english "Universal payment order"). Requirements are in https://www.upn-qr.si/uploads/files/NavodilaZaProgramerjeUPNQR.pdf document, but it is in Slovene. It has some special requirements though, which have to be met to get approval for printing QR code in UPN forms. Theses requiremenst are:

  • QR Version must be 15
  • ECC must be M
  • Byte mode
  • ECI set to 4 (ISO8859-2)
  • and of course specially formatted string

Attached is text for Wiki.

SlovenianUpnQr-md.txt

@ahotko
Copy link
Author

ahotko commented Apr 10, 2018

I have changed one parameter default value in SlovenianUpnQr and corrected the description.

SlovenianUpnQr-md.txt

@codebude
Copy link
Owner

Hi @ahotko ,

at first thanks for providing the documentation.md! As promised I had a look over the code. Before merging I have one or two questions:

Besides this two questions I'm fine with your changes. Could you help to erase my question marks?

@ahotko
Copy link
Author

ahotko commented Apr 21, 2018

Hei,

  • PlainTextToBinaryECI was ment to be for ECI Encoding, but as ECI is only for character encoding, mode doesn't matter (even better if it's byte mode), so I forgot to delete the method

  • in case of empty PlainTextToBinary case statement: PlainTextToBinaryECI shoul be used here, but I have removed it, as it is not used; case can be empty as it defaults to default (as it did before) - you can remove it or leave it, does not matter as EncodingMode.ECI is never assigned (maybe future use?)

Kind regards,
Ales

@codebude
Copy link
Owner

codebude commented Apr 21, 2018

Ok, then everything is clear. Thanks for your support. 👍

@codebude codebude merged commit 27bb1ec into codebude:master Apr 21, 2018
@codebude
Copy link
Owner

codebude commented Apr 22, 2018

@ahotko - quick note for your information. I had to slightly rewrite your code, to make it work with the PCL version of QR coder. If you need this, too, you should update from my repo.
Changes can be seen here: f3507e3

@codebude
Copy link
Owner

Hi @ahotko I just released version 1.3.3 on Nuget. So your payload generator is part of the official Nuget package now.

@ahotko
Copy link
Author

ahotko commented Apr 22, 2018

Hei @codebude - thanks for info and merge! Have a great day!

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.

None yet

2 participants