migrate plugin to machine registry integration #46
Conversation
simonkuehling
force-pushed
the
machine-registry-integration
branch
from
fb81774 to
28d62af
Compare
simonkuehling
force-pushed
the
machine-registry-integration
branch
from
28d62af to
92ec154
Compare
|
Hi @simonkuehling I'm going to have to rely on you for testing this - currently do not have access to one of these printers. Can you provide confirmation / evidence that this works for multiple connected printers? |
|
Yes, i tested with multiple (3 different models: QL-820NWB, QL-720NW, QL-1110NWB) printers installed at once and will run this setup in production after release as well 👍 |
|
@simonkuehling nice. I'm happy to merge then, it would be great if you can support any fixes that need to be implemented around this in the future (if anything arises) |
|
Sure, no problem! |
|
@simonkuehling it would be great if you could provide some updated documentation for the plugin (in README.md) on how to setup a "machine" instance for a brother label printer given the new approach |
|
|
||
| # Get specifications of media type | ||
| media_specs = None | ||
| for label_specs in ALL_LABELS: | ||
| if label_specs.identifier == media_type: | ||
| media_specs = label_specs | ||
|
|
||
| rotation = int(self.get_setting('ROTATION')) + 90 | ||
| rotation = int(machine.get_setting('ROTATION', 'D')) + 90 |
There was a problem hiding this comment.
@simonkuehling this line change has started to cause some issues:
Obviously D is not a valid integer value - can you expand on what the thinking was here?
There was a problem hiding this comment.
Following from the inventree issue. @SchrodingersGat this is not a default value, the D means get the ROTATION settings from the Driver provided setting specific to that machine and not the machine type provided settings. See https://docs.inventree.org/en/stable/extend/machines/overview/#machine.BaseMachineType.get_setting
There was a problem hiding this comment.
Ok, my mistake! Still, looks like this line needs some error handling added
There was a problem hiding this comment.
Normally if everything in inventree works, this should be treated as "error handled" already, as per setting defitition a default value is specified and a int validator is applied so there is no chance that this is something different. The issue lays it inventree. We need to somehow disable the machine feature completely if no shared cache is present. Because if there is no shared cache, the machine instance is not available in the worker => the settings definition for that machine is not there, the get_setting method does not know what this setting is about => it returns "". Ideally, the int() in this plugin is unnecessary if everything in core works correctly.
There was a problem hiding this comment.
That makes sense. We already have the is_global_cache_enabled method which we use to set the number of background workers, and a few other small caching tweaks. So, that could be used here.
changes the plugin structure to register as a machine driver in the machine registry - now we can run multiple Brother label printers (of different models as well) in one InvenTree installation.
Implements #32