Skip to content

Add basic support for scanning vendor barcodes#5509

Merged
SchrodingersGat merged 72 commits intoinventree:masterfrom
30350n:vendor_barcodes
Oct 19, 2023
Merged

Add basic support for scanning vendor barcodes#5509
SchrodingersGat merged 72 commits intoinventree:masterfrom
30350n:vendor_barcodes

Conversation

@30350n
Copy link
Contributor

@30350n 30350n commented Sep 5, 2023

This adds 2 simple (and very similar) builting plugins for scanning digikey and mouser 2D barcodes.

The code parses the barcode data fields and return a matching SupplierPart, based on SKU (for digikey) and MPN + supplier name (for mouser, because there sadly is no SKU present in the Mouser barcodes).

Related to #5119 (though not providing a full implementation) and probably also #3261.

@netlify
Copy link

netlify bot commented Sep 5, 2023

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit f15dcfc
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/652ff12335aca4000888c32d

@matmair
Copy link
Member

matmair commented Sep 5, 2023

looks great @30350n! Do you have a mouser packaging label you could provide for testing? I only have Digikey shipping labels available.

@matmair matmair added enhancement This is an suggested enhancement or new feature barcode Barcode scanning and integration labels Sep 5, 2023
@matmair matmair modified the milestones: 0.12.7, 0.13.0 Sep 5, 2023
@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

looks great @30350n! Do you have a mouser packaging label you could provide for testing? I only have Digikey shipping labels available.

Sure, here's two!

Mouser Barcode 1

Barcode Data: [)>�06�KINC-2023-03-28-00001�14K011�1PMC34063ADR�Q3�11K073124249�4LMX�1VTI��
(INC-2023-03-28-00001 is a custom order number I provided during checkout).

Mouser Barcode 2

Barcode Data: >[)>06�K21420552�14K033�1PLDK320ADU33R�Q32�11K060938769�4LCN�1VSTMicro

And finally here's a stone old (2017 or 2018 iirc) LCSC QR code (haven't implemented a handler for that one yet):

Barcode Data: {pbn:PICK2009290002,on:SO2009290140,pc:C312270,pm:ST-1-102-A01-T000-RS,qty:2,mc:,cc:1,pdi:34421807}

If anyone has a more recent one from LCSC, I'd be happy to take a look at implementing a handler!

@wolflu05
Copy link
Member

wolflu05 commented Sep 5, 2023

This is really cool, and I'm sure many InvenTree users use mouser and digikey, but I'm not sure if this should be part of InvenTree core as not everybody is using InvenTree for electronics with those two vendors. I would suggest publish this as a separate plugin and maybe combine it in some time with an import function if this plugin feature is available, so that there are one plugin for each vendor handling, barcodes, imports and possibly also other things.

@matmair
Copy link
Member

matmair commented Sep 5, 2023

I disagree, including some batteries to get to a functioning system faster seems better then having to explain how to install plugins for the n th time.
There are no external dependencies so I do not see any downsides if we are including it.

@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

I also think this is minimal and useful enough to include as a builtin plugin. You can also always disable it or disable plugins all together if you don't need the functionality. (Though admittedly the main reason for implementing this as a builtin plugin for me was because it seemed to be the easiest ^^)

I could see this getting kind of bloaty with one addon per vendor though, if I were to add even more vendors (LCSC, Farnell, RS-Components etc. come to mind).
@matmair do you think it'd be a good idea to leave this as a single plugin which could handle all kinds of vendor barcodes, or do you prefer to leave it as is?

@matmair
Copy link
Member

matmair commented Sep 5, 2023

@30350n I prefer separate plugins.

Aslo Mouser Barcode 1 seems to match a Digikey barcode?

@wolflu05
Copy link
Member

wolflu05 commented Sep 5, 2023

I would say this is an argument to improve the plugin installing system then. I agree, currently it is to complicated: searching via google or the plugin page on the website for a specific plugin, copy the package name and paste it to InvenTree and install it and maybe also restart inventree several times while running migrations or collect static inbetween. This should all be integrated into InvenTree like octoprint has it. In the end I imagine installing a plugin is a one click task after searching for the plugin directly in the admin ui.

We need some guidelines where we make a cut and don't install/ship it by default. I'm sure many don't want to clutter their plugin list with unnecessary builtins that cannot be removed neither disabled. And in my opinion mouser and digikey are some specific vendors. We need to make a cut somewhere, because otherwise some other people come and implement reichelt, rs, farnell, ... as builtins and then we end up with a long list of plugins by default, not everyone is using which makes the whole modular plugin system useless in my opinion.

  1. I would suggest creating an inventree/inventree-plugins repository then and make a folder for each of these "official" plugins and publish them individually as pip packages.
  2. I would suggest improving the plugin list a bit so that it can be fetched via some kind of api and directly integrated into InvenTree admin ui like octoprint, nodered and iobroker have it.
  3. I would suggest that if the import Mixin is there, these plugins also provide official support for importing parts and maybe also syncing orders and pricing and much more integrations.

@matmair
Copy link
Member

matmair commented Sep 5, 2023

this is an argument to improve the plugin installing system

You are very welcome to do that. Octoprint is written in flask which is very different from Django implementation-wise.

All the suggestions sound good but I would rather ship something that just works now than dream about something like #3261 for a year.

@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

@30350n I prefer separate plugins.

Aslo Mouser Barcode 1 seems to match a Digikey barcode?

Huh, yeah, right, I actually didn't test with that one, but the second one. 1. is newer though, so I guess they decided to use the same "header" or "magic" or whatever as digikey ... which kind of makes them more complicated to distinguish.

@matmair
Copy link
Member

matmair commented Sep 5, 2023

@30350n might that be an industry standard?

Copy link
Member

@matmair matmair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well overall; before this can be merged we should address 2 things:

  • adding a few sentences to the docs about this detection feature
  • adding test coverage to ensure this does not break in refactorings etc.

@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

We need some guidelines where we make a cut and don't install it by default. I'm sure many don't want to clutter their plugin list with unnecessary builtins that cannot be removed neither disabled.

But you can disable builtin plugins though, right? I kind of get your point, but i feel like the solution would be to simply disable all builtin plugins by default (which kind of seems to be the case already?).

This should all be integrated into InvenTree like octoprint has it. In the end I imagine installing a plugin is a one click task after searching for the plugin directly in the admin ui.

Yes, but that kind of system doesn't exist yet and once it does exist, it'll be trivial to integrate these plugins into it.

I would suggest that if the import Mixin is there, these plugins also provide official support for importing parts and maybe also syncing orders and pricing.

I agree that this is probably a sensible way to move forward, but imo this is another argument for having these plugins as builtins. Because that way if other people would like to contribute new vendors or part importing or similar things, there'd be a central place for that code to exist instead of it being hidden away and scattered across multiple small repos all over the place.

@wolflu05
Copy link
Member

wolflu05 commented Sep 5, 2023

All the suggestions sound good but I would rather ship something that just works now than dream about something like #3261 for a year.

So you're not against moving this out into a own package that is not installed by default if such a one-Click install flow I proposed above is available? Then I would be fine with merging this but move this out at a later point.

@matmair
Copy link
Member

matmair commented Sep 5, 2023

All the suggestions sound good but I would rather ship something that just works now than dream about something like #3261 for a year.

So you're not against moving this out into a own package that is not installed by default if such a one-Click install flow I proposed above is available? Then I would be fine with merging this but move this out at a later point.

No. But I do not anticipate any such one-click solution any time soon. I tried building this multiple times over my 5+ years with django and it is a very complex effort that IMO is not worth the effort.

@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

@30350n might that be an industry standard?

Yeah, probably. Though more like industry """standard""". I think this is some kind of quasi-standard for 2D barcodes, but the way digikey and mouser setup the data is just different enough to make it impractical to handle them together.
If you take a look at the implementation, the "per entry headers" are sometimes the same (1P and Q for MPN and quantity) and sometimes similar, but not quite the same (10K/11K for invoice number, etc.).

@wolflu05
Copy link
Member

wolflu05 commented Sep 5, 2023

But you can disable builtin plugins though, right?

If I remember correctly, they are forced to be active, but can't test currently.

across multiple small repos all over the place

That's why I proposed the third point (using an official inventree/inventree-plugins monorepo)

@matmair
Copy link
Member

matmair commented Sep 5, 2023

@wolflu05 If you do not mind I would appreciate moving the general plugin discussion to a "discussion", threading is hard in issues.

@30350n there seems to be a standard: https://www.ecianow.org/assets/docs/ECIA_Specifications.pdf - would you be up to implement/look into that?
grafik

@30350n
Copy link
Contributor Author

30350n commented Sep 5, 2023

@30350n there seems to be a standard: https://www.ecianow.org/assets/docs/ECIA_Specifications.pdf - would you be up to implement/look into that?

Sure! Haven't seen that one yet, but that seems to be relatively straight forward.
Vendors using this barcode format would still possibly have to be handled differently though, so the two plugins should stay right?
I think I'd personally just implement this as a single function which parses a "ECIA 2D Barcode", but where would this function live? Should I add something like a ecia_2d_barcode.py to plugin/base/barcodes?

@matmair
Copy link
Member

matmair commented Oct 12, 2023

Looking good

@SchrodingersGat
Copy link
Member

@30350n nice progress - a few more things I'd like to see addressed :) I have not had time to test your updated code with a real barcode and scanner, but if you can ping me when you've made the changes as above I'll check it out :)

@30350n
Copy link
Contributor Author

30350n commented Oct 17, 2023

Thanks for reviewing again! :)

Copy link
Member

@SchrodingersGat SchrodingersGat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, still a couple of issues. In particular, the classes must all inherit from SettingsMixin

@SchrodingersGat
Copy link
Member

@30350n getting very close! One more change, see notes

@30350n
Copy link
Contributor Author

30350n commented Oct 19, 2023

I still don't really get the receive_line_item thing, sorry. Can you elaborate on that?

@SchrodingersGat
Copy link
Member

The receive_line_item method may conceivably throw an error, but you don't want that to break the process. So, something like:

try:
   receive_line_item()
except Exception:
   # Return an error message to the user
   error = _("Line item could not be received")

@30350n
Copy link
Contributor Author

30350n commented Oct 19, 2023

Yeah, but the receive_line_item should not throw an error unless there's an actual bug in the code, right?
In which case you want to get an Exception.

As I wrote before:

The way I understand it, if some InvenTree code throws an Exception you get one of those error notifications and an entry in the admin panel. Imo, this is exactly what should happen here. If receive_line_item throws an error, there is a bug in the code which has to be fixed. Thus, if we were to wrap it and throw a "soft" error, it would seem as if it's an expected error (you'd also get no tracebacks and it'd just generally make everything much harder to debug).

Sorry, but this still seems like an anti feature to me.

@SchrodingersGat
Copy link
Member

@30350n sorry, I must have missed your previous comment, and you make a good argument. On balance I think your approach is correct and thus I think we are good to merge!

@SchrodingersGat
Copy link
Member

@30350n thanks for the huge effort here, this is a great new feature! Please keep a lookout for any bugs, and if you can spare some time soon for the associated documentation, that would be great :)

@SchrodingersGat SchrodingersGat merged commit ae063d2 into inventree:master Oct 19, 2023
@30350n
Copy link
Contributor Author

30350n commented Oct 19, 2023

@30350n sorry, I must have missed your previous comment, and you make a good argument. On balance I think your approach is correct and thus I think we are good to merge!

All good, I kinda figured, but I didn't want to be pushy and bring it up again ^^

Thanks a lot for merging! I'll keep an eye out for any issue emerging and possible improvements too :)

Also have to say that it's been a blast working together with you guys, communication was really great! Thanks for all the feedback again.

@SchrodingersGat
Copy link
Member

@30350n it has been a pleasure working with you too - please keep contributing!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

barcode Barcode scanning and integration enhancement This is an suggested enhancement or new feature feature plugin Plugin ecosystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants