pos icon indicating copy to clipboard operation
pos copied to clipboard

[14.0][MIG] pos_customer_display and add support for Epson ePOS for displays

Open alexis-via opened this issue 2 years ago • 7 comments

I migrated the pos_customer_display module to v14 and I added support for the ePOS display protocol.

This module now supports 2 solutions for customer display:

  1. USB LED LCD connected to IoTBox or pywebdriver,
  2. Epson ePOS protocol for displays: if you have an Epson USB display (DM-D30 for example) connected to an Epson IP printer that support ePOS (TM-m30 for example) and has ePOS print enabled in it's web configuration interface, Odoo can connect directly to it. The JS code of the POS will send the IP request to the ePOS printer that will relay the information to the USB display.

Advantages of ePOS vs IoTBox/pywebdriver:

  • supports accentuated caracters
  • no need to install/setup IoTBox/pywebdriver

I added a dependency on pos_epson_printer, but it should not be a real problem because it is a very small module of the official addons and it won't bother you if you don't use it.

Use 'x' instead of '*' for multiple quantities.

Superseeds #756

alexis-via avatar Aug 04 '22 20:08 alexis-via

Here is the config for ePOS:

pos_config_epos_v2

alexis-via avatar Aug 04 '22 21:08 alexis-via

hi @alexis-via. Thanks a lot for this great job !

I'm a quite reluctant regarding the design of the module. (having 1 module for two kind of protocols). In a first sight it's great, but I fear that it generates problem during migration for developpers. if a contributor want to migrate the module in V16, but has only ePOS display, he will not test the USB LED prototocol, and maybe the module will not work at all for the second protocol. and it forces to install useless piece of code. Also maybe in 5 years, a protocol will be fully obsolete. If we have two modules, the obsolete protocol will be dropped because unported. Otherwise, we will keep useless base code.

An alternative design could be to have :

  • a generic module pos_customer_display_abstract that implement generic field (text on pos.config and a field.selection iface_customer_display_type and generic Javascript code.
  • a module 1 pos_customer_display_usb_led for the historical protocol.
  • another module pos_customer_display_epos that extra depends on pos_epson_printer.

As #756 and 578 exist since 2 years, for sure many people use the current module in production. So we could plan to do this refactor for V15 / V16.

What do you think ? CC : @ivantodorovich any point of view ?

legalsylvain avatar Aug 05 '22 07:08 legalsylvain

I agree with @legalsylvain , specially because the module dependencies have changed from point_of_sale to pos_epson_printer. And that last one might not be needed and not wanted for most people

ivantodorovich avatar Aug 05 '22 14:08 ivantodorovich

@legalsylvain I agree that the best solution is to have 3 modules, as described in your last comment. I don't plan to do it myself (at least not in the short term), but I will support any developers that work on this split.

I don't think the module pos_customer_display is used by a lot a people in v14 because it has never been merged. From my experience, only very experienced Odoo developers look at pull requests to find modules for a particular version ; most people just stick to the "OCA/14.0" branch. I often receive emails asking "When do you plan to port module xxx to v14?" and I have to reply "I already ported that module to v14, you will find it on this PR <URL_to_PR>.".

I know that this PR adds an extra dependency, on pos_epson_printer. pos_epson_printer is a very small module and it's not a real problem to have it if you don't use it. That's why I proposed this PR anyway. I think the "direct" approach without pywebdriver/POSbox will rapidly replace the old pywebdriver/POSbox solution, at least on new deployments (for existing deployments, it will certainly take more time because it requires replacing POS hardware by some new hardware). On twitter, Sylvain mentioned the scales that only have a serial port : for this kind of hardware, it's possible to use an RS232 to IP dongle such as USR-TCP232-306 (you can find it for less that 30 € without taxes here for example https://www.soselectronic.fr/products/usr-iot/usr-tcp232-306-321365). I haven't tested these RS232 to IP converters, so I cannot confirm it works.

In the end:

  • having 3 modules is the best technical approach. But it needs some extra work to do the split.
  • having 1 module is maybe an acceptable and pragmatic solution, at least until someone starts to work on the split.

alexis-via avatar Aug 10 '22 08:08 alexis-via

+1 @alexis-via I also think that we need to stay as close as possible to 'real life'... How many people use these modules ? It should be a lot less than we think...

cvinh avatar Aug 10 '22 08:08 cvinh

Maybe this could be merged with a more detailed 'Roadmap/todo'

cvinh avatar Aug 10 '22 08:08 cvinh

In the end: having 3 modules is the best technical approach. But it needs some extra work to do the split. having 1 module is maybe an acceptable and pragmatic solution, at least until someone starts to work on the split.

Totally agree !

In a few words : On this PR V14 :

  • add a ROADMAP.rst file that says more or less "TODO : split into 3 modules" with a detailled text and a link to the comments we did. could you do it ?
  • Then, let's merge this PR as is.

For V15 / V16 :

  • I'll target to migrate to 16, and the refactoring is not such a big deal I think, so I can do it. if we can synchronize us it's great, because I'll have to test the other device I don't have. We can plan to do this job at the next OCA days, if you come ?

OK for all people ?

I think the "direct" approach without pywebdriver/POSbox will rapidly replace the old pywebdriver/POSbox solution

On that topic, I'm not sure. Fortunately, some manufacturers are not in a logic of programmed obsolescence. This is the case of Epson. I've been using Odoo (+ Pywebdriver) for 8 years, and the oldest USB printers still work. Maybe they will work for another 10 or 15 years (and personally, I hope so to preserve the resources of our dear planet!) The same for the LED display. So, I am convinced that I will migrate the pos_customer_display_usb_led module to version 24 in a few years. ;-)

legalsylvain avatar Aug 10 '22 09:08 legalsylvain

Something I forgot to mention: the module pos_epson_printer from the official addons has auto_install = True, cf https://github.com/odoo/odoo/blob/14.0/addons/pos_epson_printer/manifest.py#L21

alexis-via avatar Oct 22 '22 21:10 alexis-via

Hi @alexis-via. this PR is ready to merge. could you just remove some console.log that makes CI red ? thanks !

legalsylvain avatar Mar 13 '23 22:03 legalsylvain

hi @alexis-via any chance that you find some time to make the change requested by @legalsylvain ? many thanks !

houssine78 avatar Jun 20 '23 10:06 houssine78

@houssine78 can you finish the work ? (Making another PR?)

legalsylvain avatar Jun 20 '23 10:06 legalsylvain

still a minor error in pre-commit. https://github.com/OCA/pos/actions/runs/5322807409/jobs/9639755631?pr=824#step:7:209

legalsylvain avatar Jun 20 '23 12:06 legalsylvain

Note : I'll begin to migrate this module in the next weeks, working on the split in 3 modules.

legalsylvain avatar Jun 20 '23 12:06 legalsylvain

Thanks ! /ocabot merge patch

legalsylvain avatar Jun 20 '23 13:06 legalsylvain

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 14.0-ocabot-merge-pr-824-by-legalsylvain-bump-patch, awaiting test results.

OCA-git-bot avatar Jun 20 '23 13:06 OCA-git-bot

Congratulations, your PR was merged at 6d3868a75fe829dca5cb8b1e87c05ecd66a14bf1. Thanks a lot for contributing to OCA. ❤️

OCA-git-bot avatar Jun 20 '23 13:06 OCA-git-bot

thanks a lot @alexis-via

houssine78 avatar Jun 20 '23 21:06 houssine78