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

Development of Initial CUSTOM Printer Implementation #105

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jmalda
Copy link

@jmalda jmalda commented Feb 3, 2021

Created initial framework for supporting the CUSTOM Printer implementation of ESC \ POS Protocol.

Support has been added for CUSTOM QR Code printing. Printout on left is is EPSON protocol on a CUSTOM Printer. Printout on right is CUSTOM protocol on a CUSTOM printer.

QR Code Print Test

@jmalda
Copy link
Author

jmalda commented Feb 3, 2021

FYI @danielmeza @igorocampos

@igorocampos
Copy link
Collaborator

Wow nice to see the 2D barcodes printed! I've implemented it but couldn't test it effectively.

@igorocampos
Copy link
Collaborator

@jmalda nice job implementing this! Are all the other commands (alignment, text size, image printing, etc) the same as EPSON?

@igorocampos
Copy link
Collaborator

igorocampos commented Feb 3, 2021

@jmalda sorry about the flood, but I was thinking now, maybe we can centralize a good chunk of Print2DCode and override only the little part that differs from EPSON? I mean, creating new method(s) that share the boilerplate kinda. Just a thought.

Or what if we just changed these byte arrays to be virtual and thus override only them ?
image

@igorocampos
Copy link
Collaborator

@jmalda I promise this is the last time! lol

Can you please add the new emitter to the Global Unit tests? You need just to add the same InlineData as EPSON's on both tests and replace EPSON by CUSTOM
image

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

@jmalda nice job implementing this! Are all the other commands (alignment, text size, image printing, etc) the same as EPSON?

Thanks @igorocampos . The only commands I noticed so far that were slightly different were the QR Code implementations. I will print the various tests from the Console App again tomorrow and see if there is anything else that jumps out.

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

@jmalda I promise this is the last time! lol

Can you please add the new emitter to the Global Unit tests? You need just to add the same InlineData as EPSON's on both tests and replace EPSON by CUSTOM
image

@igorocampos Will do. I did notice a number of the current tests appear to fail because the barcode content is longer than 21 characters.

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

@jmalda sorry about the flood, but I was thinking now, maybe we can centralize a good chunk of Print2DCode and override only the little part that differs from EPSON? I mean, creating new method(s) that share the boilerplate kinda. Just a thought.

Or what if we just changed these byte arrays to be virtual and thus override only them ?
image

@igorocampos No worries about the flood of messages. 👍

Are you thinking along the lines of an ICommandValue interface, with a base (EPSON) implementation, and a CUSTOM implementation that only overrides the StoreQRCodeData and PrintQRCode properties? And then each Emitter implementation would then have an internal reference to its "own" implementation of ICommandValue?

@lukevp
Copy link
Owner

lukevp commented Feb 4, 2021

Thanks for working on this @jmalda ! It looks like the QR codes are the same though? Does the CUSTOM not support the different QR types?

@igorocampos
Copy link
Collaborator

Are you thinking along the lines of an ICommandValue interface, with a base (EPSON) implementation, and a CUSTOM implementation that only overrides the StoreQRCodeData and PrintQRCode properties? And then each Emitter implementation would then have an internal reference to its "own" implementation of ICommandValue?

@jmalda yes, something like that. I didn't think it all the way through, I was just throwing ideas of how can we reduce the number of repetitive code lines, you know?

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

Thanks for working on this @jmalda ! It looks like the QR codes are the same though? Does the CUSTOM not support the different QR types?

Hi @lukevp ! I will compare the ESPON and CUSTOM commands in their respective protocol documentation to see if there are any differences regarding the QR Code types. Are you able to share with me what a printout of the 2D Codes looks like on an ESPON printer?

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

Are you thinking along the lines of an ICommandValue interface, with a base (EPSON) implementation, and a CUSTOM implementation that only overrides the StoreQRCodeData and PrintQRCode properties? And then each Emitter implementation would then have an internal reference to its "own" implementation of ICommandValue?

@jmalda yes, something like that. I didn't think it all the way through, I was just throwing ideas of how can we reduce the number of repetitive code lines, you know?

@igorocampos It would certainly reduce duplicate code, and also provide greater flexibility for allowing overrides at both the CommandEmitter and CommandValues level. Please let me know if you want that as part of this PR or not. Thanks.

@jmalda
Copy link
Author

jmalda commented Feb 4, 2021

@jmalda nice job implementing this! Are all the other commands (alignment, text size, image printing, etc) the same as EPSON?

Thanks @igorocampos . The only commands I noticed so far that were slightly different were the QR Code implementations. I will print the various tests from the Console App again tomorrow and see if there is anything else that jumps out.

@igorocampos Below are most of the current Print Tests being printed on the CUSTOM printer. It looks like there are a few issues with the GS1 Barcodes, as well as Images (Legacy Images appear to work just fine).

CUSTOM Print Tests

@igorocampos
Copy link
Collaborator

@igorocampos It would certainly reduce duplicate code, and also provide greater flexibility for allowing overrides at both the CommandEmitter and CommandValues level. Please let me know if you want that as part of this PR or not. Thanks.

@jmalda I'd say yes for that, BUT who am I to decide? I'm just a contributor like you :) @lukevp is the owner of this repository

@danielmeza
Copy link

¡Hi! sorry for join late on the conversation, you did grate! @jmalda. @igorocampos now that we have virtualized the methods on the BaseCommandEmitter we can start here to reduce the boilerplate code, another thing we can do is definitely an interface to hide the real implementation and to work better with DI or IoC

@jmalda
Copy link
Author

jmalda commented Feb 5, 2021

@igorocampos It would certainly reduce duplicate code, and also provide greater flexibility for allowing overrides at both the CommandEmitter and CommandValues level. Please let me know if you want that as part of this PR or not. Thanks.

@jmalda I'd say yes for that, BUT who am I to decide? I'm just a contributor like you :) @lukevp is the owner of this repository

Hi @danielmeza @igorocampos I started down the road of changing the CommandValues to adhere to some sort of interface. It is a significant change with many files affected. I am wondering if this is something we want to do as a completely separate PR so that it contains just this refactor. @lukevp Thoughts?

@lukevp
Copy link
Owner

lukevp commented Feb 15, 2021

Agreed that refactoring the CommandValues to have an interface would be a separate PR. I'd like to see if you can figure out why the different QR Code types don't look right before we merge this PR, as you're the only one who has this brand of printer. Here is what the QR code types look like on Epson:
2_1_21, 8_39 AM Office Lens 6

@lukevp
Copy link
Owner

lukevp commented Feb 15, 2021

Can you all think of a way we could hide / shadow the non-legacy images? It appears to only work on EPSON, as the CUSTOM and other printers we've seen come through here all fail to print the non-legacy image test. I think we should move the non-legacy image implementation out of the base implementation and only into the EPSON emitter if that's the case.

This might also be a good approach with the GS1 barcodes, if most printers don't support them perhaps they should be only in EPSON?

@igorocampos
Copy link
Collaborator

I think we should move the non-legacy image implementation out of the base implementation and only into the EPSON emitter if that's the case.

I totally agree with moving it out of base emitter, and have it implemented only in EPSON's emitter. The alternative would be a more basic base emitter, that has only simple commands that we all know that successfully runs in all printers, then a base epson-like class with a little bit more commands that other generic printers don't accept, but EPSON and others do.

However, I'm not sure how true that would be, if there are any "epson-like" printers. That's why I would go for what @lukevp first said, just take it out of base and implement directly in EPSON.

@jmalda
Copy link
Author

jmalda commented Feb 15, 2021

Agreed that refactoring the CommandValues to have an interface would be a separate PR. I'd like to see if you can figure out why the different QR Code types don't look right before we merge this PR, as you're the only one who has this brand of printer. Here is what the QR code types look like on Epson:...

The more I dig into this, the more differences I find between the EPSON and CUSTOM implementations of ESC \ POS. For starters, CUSTOM does not take advantage of any 100 series functions. I.e. where ESPON uses Function 167, CUSTOM uses Function 067 and so on. Digging into the barcodes, it looks like some of our underlying enums would need to change as well depending on the implementation (e.g. Size2DCode, TwoDimensionCodeType etc). I will continue to get to the bottom of the barcodes. If you are interested, below are the EPSON, CUSTOM, and STAR Micronics command manuals for ESC \ POS.

CUSTOM ESC POS Command Manual.pdf
EPSON ESC POS Command Manual.pdf
STAR Micronics ESC POS Command Manual.pdf

@danielmeza
Copy link

For Enums we can use SmartEnum or a custom implementation of Enhanced Enums

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

4 participants