From c90ebcbc7c055eb172cc9e1fa39e2aa0a507d975 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Tue, 11 Aug 2026 11:13:12 -0400 Subject: [PATCH] computers: declare subordinate devices instead of coding each one A PC that drives a device which is its own asset had been implemented twice. METROLOGY_TOOL_MAP covered CMM, Keyence, Genspect and wax-trace, minting a measuring_tool. A separate path keyed on one hardcoded pc-type minted a Part Marker machine and filed it under its operation. Both create a device, link the PC with controls, and archive that link when the PC is re-imaged: one mechanism with different nouns, written out twice because the second case arrived later. That is the same trap as the site literals in ADR-015 - a pattern implemented per instance rather than declared - and it has a known next occurrence. Part markers already share operation numbers, and any site with two marking lasers or two wax-trace units on one number needs identical treatment. One SUBORDINATE_DEVICE_MAP now declares asset type, type name, naming suffix, whether the device files partof the operation, and the relationship label. The labels are unchanged per case on purpose: those values are in the production database and only rows carrying them are archived by a collector push. A site overrides or adds an entry through subordinatedevice_ settings, per ADR-015, so the next case needs no code. A malformed override falls back to the default rather than failing the push, because a bad setting must not stop a bay reporting its inventory. metrology_tool_for stays as a shim over the same map: filters.py and the older tests read it, and unifying must not change what it returns. A test pins that. Also adds flask relationships check-shared-machines, which finds the next 0615 rather than waiting for someone to notice duplicate backups. Several devices legitimately sharing a number and two PCs mis-numbered at imaging look the same from outside; the difference is whether child assets exist, so that is what it reports. Read-only. --- CHANGELOG.md | 15 +++ plugins/computers/pctypemap.py | 117 ++++++++++++++++----- plugins/computers/plugin.py | 60 ++++++----- shopdb/cli/__init__.py | 85 +++++++++++++++ tests/test_core/test_collector_contract.py | 56 ++++++++++ 5 files changed, 280 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ebcea7..f733876 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,21 @@ ADR-007 and ADR-002. `computers_machinelink_alerts` setting that ships OFF: a site may legitimately run several PCs on one machine number, and there the alerts fire on correct data. The collector response warns either way. +- A PC that drives a subordinate device is now declared, not coded. Two cases + had arrived separately and been written twice: a metrology PC drives a CMM or + Keyence unit that becomes a measuring-tool asset, and a part-marker PC drives + a marker that becomes a machine asset filed under its operation. Both mint a + device, link the PC with `controls` and archive that link on a re-image - one + mechanism with different nouns. They are now one map, and a site can add or + retarget an entry through a `subordinatedevice_` setting, so the next + case - two marking lasers or two wax-trace units on one machine number - is + configuration rather than a code change. A malformed override falls back to + the default instead of failing the collector push. +- `flask relationships check-shared-machines` reports machine numbers claimed + by more than one PC, and says which are modelled and which are faults. Several + devices genuinely sharing a number and two PCs mis-numbered at imaging look + identical from the outside; the difference is whether anything is filed under + the operation, which is what the check reports. Read-only. - A part marker is its own asset. Several Telesis markers can serve one operation number, so treating the operation as the marker collapsed separate devices into one record: their configurations, which differ by COM port, diff --git a/plugins/computers/pctypemap.py b/plugins/computers/pctypemap.py index 94e74f4..6a6894f 100644 --- a/plugins/computers/pctypemap.py +++ b/plugins/computers/pctypemap.py @@ -29,40 +29,103 @@ DEFAULT_PCTYPE_MAP = { _SETTING_PREFIX = 'pctypemap_' _SETTING_CATEGORY = 'pctypemapping' -# Metrology imaging pc-types: the PC itself stays a shopfloor PC, but it drives -# an attached measuring instrument. When the collector sees one of these, it -# auto-creates a MeasuringTool asset for the device and links the PC to it with -# a "controls" relationship (see ComputersPlugin._sync_measuringtool_link). -# Maps imaging pc-type -> (MeasuringToolType name, type description). The type -# is created on demand if the measuringtools plugin has not seeded it. -METROLOGY_TOOL_MAP = { - 'gea-shopfloor-cmm': ('CMM', 'Coordinate measuring machine'), - 'gea-shopfloor-keyence': ('Vision System', 'Optical / vision measurement system (Keyence)'), - 'gea-shopfloor-genspect': ('Genspect', 'Genspect visual / borescope inspection system'), - 'gea-shopfloor-waxtrace': ('Form Tracer', 'Surface / form tracing system (wax and trace)'), +# SUBORDINATE DEVICES: a PC that drives a device which is its own asset. +# +# Two cases arrived separately and were written twice. A metrology PC drives a +# CMM or a Keyence unit, which is a measuring_tool asset. A part-marker PC +# drives a Telesis marker, which is a machine asset filed under the operation it +# serves. Both mint a device, link the PC to it with `controls`, and archive the +# link if the PC is re-imaged - the same mechanism with different nouns. +# +# Declared here rather than coded per case so the NEXT one is configuration. It +# will happen: several markers already share operation numbers 0613, 0615 and +# WJPRT, and any site with two marking lasers or two wax-trace units on one +# number needs the same treatment. A site adds or retargets an entry through the +# subordinatedevice_ settings (category 'pctypemapping'), per ADR-015. +# +# Fields: +# assettype the core AssetType. 'measuring_tool' or 'machine' today; the +# extension row and type vocabulary follow from it. +# typename the device type within that vocabulary, created on demand. +# description used only when the type is created. +# suffix appended to the PC's asset number to name the device. +# partof True files the device `partof` the reported operation AND stops +# the PC claiming that operation directly. Set it when several +# devices can share one machine number: `controls` propagates +# along `partof`, so control still follows, without two devices +# contesting a link only one can hold. +# label relationship origin marker. UNCHANGED per case on purpose - +# these values are in the production database and only rows +# carrying them are archived by a collector push. +SUBORDINATE_DEVICE_MAP = { + 'gea-shopfloor-cmm': { + 'assettype': 'measuring_tool', 'typename': 'CMM', + 'description': 'Coordinate measuring machine', + 'suffix': 'CMM', 'partof': False, 'label': 'collector:measuringtool', + }, + 'gea-shopfloor-keyence': { + 'assettype': 'measuring_tool', 'typename': 'Vision System', + 'description': 'Optical / vision measurement system (Keyence)', + 'suffix': 'KEYENCE', 'partof': False, 'label': 'collector:measuringtool', + }, + 'gea-shopfloor-genspect': { + 'assettype': 'measuring_tool', 'typename': 'Genspect', + 'description': 'Genspect visual / borescope inspection system', + 'suffix': 'GENSPECT', 'partof': False, 'label': 'collector:measuringtool', + }, + 'gea-shopfloor-waxtrace': { + 'assettype': 'measuring_tool', 'typename': 'Form Tracer', + 'description': 'Surface / form tracing system (wax and trace)', + 'suffix': 'WAXTRACE', 'partof': False, 'label': 'collector:measuringtool', + }, + 'gea-shopfloor-partmarker': { + 'assettype': 'machine', 'typename': 'Part Marker', + 'description': 'Telesis part marker', + 'suffix': 'PARTMARKER', 'partof': True, 'label': 'collector:partmarker', + }, } - -# Imaging pc-type for a PC that drives a Telesis part marker. The marker is its -# own asset, not a field on the PC: several markers can serve one operation -# number (0613, 0615, WJPRT all have more than one), so filing their configs -# under the operation collapsed separate devices into one record. One marker per -# PC, so the PC identifies the marker - the same shape as METROLOGY_TOOL_MAP. -PARTMARKER_PCTYPE = 'gea-shopfloor-partmarker' - -# MachineType a collector-created marker is given. -PARTMARKER_TYPENAME = 'Part Marker' +_DEVICE_SETTING_PREFIX = 'subordinatedevice_' -def drives_partmarker(pctype): - """True when this imaging pc-type means the PC drives a part marker.""" - return (pctype or '').strip() == PARTMARKER_PCTYPE +def subordinate_device_for(pctype): + """Device spec for this imaging pc-type, or None. + + Site overrides live in settings named subordinatedevice_ holding a + JSON object with the same fields; a site can retarget an existing entry or + add a pc-type of its own without a code change. A malformed override is + ignored in favour of the default rather than failing the whole collector + push - a bad setting must not stop a bay reporting its inventory. + """ + key = (pctype or '').strip() + if not key: + return None + spec = SUBORDINATE_DEVICE_MAP.get(key) + setting = Setting.query.filter_by( + key='{}{}'.format(_DEVICE_SETTING_PREFIX, key)).first() + if setting and (setting.value or '').strip(): + import json + try: + override = json.loads(setting.value) + if isinstance(override, dict): + spec = dict(spec or {}, **override) + except (ValueError, TypeError): + pass + if not spec or not spec.get('assettype') or not spec.get('typename'): + return None + return spec def metrology_tool_for(pctype): - """Return (typename, typedescription) if this pc-type drives a measuring - tool, else None.""" - return METROLOGY_TOOL_MAP.get((pctype or '').strip()) + """Back-compat shim: (typename, description) for a measuring-tool pc-type. + + Kept because the parity harness and the older tests read it. New code asks + subordinate_device_for, which covers machine-typed devices too. + """ + spec = SUBORDINATE_DEVICE_MAP.get((pctype or '').strip()) + if not spec or spec['assettype'] != 'measuring_tool': + return None + return (spec['typename'], spec['description']) def pctype_mapping(): diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index 781f963..5b849f9 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -728,9 +728,13 @@ class ComputersPlugin(BasePlugin): ordinary machine link to run. """ from shopdb.api import AssetRelationship, RelationshipType, Asset - from .pctypemap import drives_partmarker, PARTMARKER_TYPENAME + from .pctypemap import subordinate_device_for - if not drives_partmarker(pctype) or not comp or not comp.asset: + spec = subordinate_device_for(pctype) + if not spec or not spec.get('partof') or not comp or not comp.asset: + # Only a device that FILES UNDER an operation goes through here. + # A measuring tool is a subordinate device too, but it does not + # share a machine number, so it keeps the simpler path. return [] pcasset = comp.asset @@ -744,15 +748,18 @@ class ComputersPlugin(BasePlugin): try: from plugins.machines.models import Machine, MachineType except ImportError: - warnings.append('machines plugin unavailable; part marker skipped') + warnings.append('machines plugin unavailable; {} device skipped' + .format(spec['typename'])) return [] - # Reuse this PC's existing marker before minting one, so a re-image - # never leaves a second marker behind for the same physical device. + label = spec['label'] + + # Reuse this PC's existing device before minting one, so a re-image + # never leaves a second device behind for the same physical unit. existing = AssetRelationship.query.filter( AssetRelationship.sourceassetid == pcasset.assetid, AssetRelationship.relationshiptypeid == controls.relationshiptypeid, - AssetRelationship.label == PARTMARKER_LINK_ORIGIN, + AssetRelationship.label == label, ).all() reuse = next((rel for rel in existing if rel.isactive), None) \ or (existing[0] if existing else None) @@ -761,35 +768,36 @@ class ComputersPlugin(BasePlugin): reuse.isactive = True markerasset = db.session.get(Asset, reuse.targetassetid) else: - machinetype = AssetType.query.filter_by(assettype='machine').first() - if not machinetype: - warnings.append('machine asset type missing; part marker ' - 'skipped') + coretype = AssetType.query.filter_by( + assettype=spec['assettype']).first() + if not coretype: + warnings.append('{} asset type missing; {} skipped'.format( + spec['assettype'], spec['typename'])) return [] - markertype = MachineType.query.filter_by( - machinetype=PARTMARKER_TYPENAME).first() - if not markertype: - markertype = MachineType(machinetype=PARTMARKER_TYPENAME, - description='Telesis part marker') - db.session.add(markertype) + devicetype = MachineType.query.filter_by( + machinetype=spec['typename']).first() + if not devicetype: + devicetype = MachineType(machinetype=spec['typename'], + description=spec.get('description')) + db.session.add(devicetype) db.session.flush() hostname = comp.hostname markerasset = Asset( - assetnumber='{}-PARTMARKER'.format( - pcasset.assetnumber or hostname), - name='Part Marker ({})'.format(hostname), - assettypeid=machinetype.assettypeid, + assetnumber='{}-{}'.format( + pcasset.assetnumber or hostname, spec['suffix']), + name='{} ({})'.format(spec['typename'], hostname), + assettypeid=coretype.assettypeid, statusid=1) db.session.add(markerasset) db.session.flush() db.session.add(Machine(assetid=markerasset.assetid, - machinetypeid=markertype.machinetypeid)) + machinetypeid=devicetype.machinetypeid)) db.session.add(AssetRelationship( sourceassetid=pcasset.assetid, targetassetid=markerasset.assetid, relationshiptypeid=controls.relationshiptypeid, - label=PARTMARKER_LINK_ORIGIN, + label=label, isactive=True)) # One marker per PC: archive any other collector marker link. @@ -798,14 +806,14 @@ class ComputersPlugin(BasePlugin): rel.isactive = False operation = self._link_marker_to_operation( - markerasset, machinenumber, pcasset, warnings) + markerasset, machinenumber, pcasset, label, warnings) return [{'assetid': markerasset.assetid, 'assetnumber': markerasset.assetnumber, 'operationassetid': operation}] def _link_marker_to_operation(self, markerasset, machinenumber, pcasset, - warnings): + label, warnings): """Make a marker `partof` the operation whose number its PC reports. Unlike the PC-to-machine link this does NOT contest: an operation can @@ -852,7 +860,7 @@ class ComputersPlugin(BasePlugin): links = AssetRelationship.query.filter( AssetRelationship.sourceassetid == markerasset.assetid, AssetRelationship.relationshiptypeid == partof.relationshiptypeid, - AssetRelationship.label == PARTMARKER_LINK_ORIGIN, + AssetRelationship.label == label, ).all() found = None for rel in links: @@ -866,7 +874,7 @@ class ComputersPlugin(BasePlugin): sourceassetid=markerasset.assetid, targetassetid=operation.assetid, relationshiptypeid=partof.relationshiptypeid, - label=PARTMARKER_LINK_ORIGIN, + label=label, isactive=True)) return operation.assetid diff --git a/shopdb/cli/__init__.py b/shopdb/cli/__init__.py index 2355e1d..c53d092 100644 --- a/shopdb/cli/__init__.py +++ b/shopdb/cli/__init__.py @@ -1217,3 +1217,88 @@ def csv_import(path, directory, tablename, commit): 'Looks good: %d would be created, %d updated.' % (total_new, total_upd), fg='green', bold=True)) click.echo('Run again with --commit to apply.') + + +@relationships_cli.command('check-shared-machines') +@with_appcontext +def check_shared_machines(): + """Find machine numbers that more than one PC reports against. + + Two very different situations look identical from the outside, and both + were found the hard way rather than by asking: + + LEGITIMATE - several devices genuinely share one number. Part markers do: + 0613, 0615 and WJPRT each carry more than one, and their configurations + differ by COM port. Modelled correctly, each device is its own asset filed + `partof` the operation, so the operation has CHILD ASSETS. + + A FAULT - two PCs carrying the same machine number, usually a mistake at + imaging. Nothing is filed under the operation, the PCs contest one link, + and whichever reported last appears to own the machine. + + The difference is whether child assets exist, which is exactly what this + reports. Read-only. + """ + from shopdb.extensions import db + from shopdb.core.models import Asset, AssetRelationship, RelationshipType + from sqlalchemy.orm import aliased + + controls = RelationshipType.query.filter_by(relationshiptype='controls').first() + partof = RelationshipType.query.filter_by(relationshiptype='partof').first() + if not controls: + click.echo(click.style("No 'controls' relationship type; " + 'run flask seed reference-data.', fg='yellow')) + return + + pcasset = aliased(Asset) + machineasset = aliased(Asset) + + # Every active collector-made PC -> machine link, grouped by machine. + rows = (db.session.query(machineasset.assetid, machineasset.assetnumber, + pcasset.assetnumber) + .select_from(AssetRelationship) + .join(pcasset, AssetRelationship.sourceassetid == pcasset.assetid) + .join(machineasset, AssetRelationship.targetassetid == machineasset.assetid) + .filter(AssetRelationship.relationshiptypeid == controls.relationshiptypeid, + AssetRelationship.label == 'collector:machine', + AssetRelationship.isactive.is_(True)) + .all()) + + bymachine = {} + for assetid, machinenumber, pcnumber in rows: + bymachine.setdefault((assetid, machinenumber), []).append(pcnumber) + + shared = {k: v for k, v in bymachine.items() if len(v) > 1} + if not shared: + click.echo(click.style('No machine number is claimed by more than one PC.', + fg='green')) + return + + faults = 0 + for (assetid, machinenumber), pcs in sorted(shared.items(), key=lambda kv: kv[0][1] or ''): + children = 0 + if partof: + children = (AssetRelationship.query + .filter_by(targetassetid=assetid, + relationshiptypeid=partof.relationshiptypeid, + isactive=True) + .count()) + if children: + click.echo(' {:<10} {} PCs, {} child asset(s) - modelled'.format( + machinenumber, len(pcs), children)) + else: + faults += 1 + click.echo(click.style( + ' {:<10} {} PCs, NO child assets - {}'.format( + machinenumber, len(pcs), ', '.join(sorted(pcs))), fg='yellow')) + + click.echo() + if faults: + click.echo(click.style( + '{} machine number(s) claimed by several PCs with nothing filed ' + 'under them.'.format(faults), fg='yellow', bold=True)) + click.echo('Either the PCs are mis-numbered - fix that on the PC - or the ' + 'device type needs an entry in SUBORDINATE_DEVICE_MAP so each ' + 'device becomes its own asset.') + else: + click.echo(click.style('Every shared number has child assets.', fg='green')) diff --git a/tests/test_core/test_collector_contract.py b/tests/test_core/test_collector_contract.py index 71e3d25..d2c9dbb 100644 --- a/tests/test_core/test_collector_contract.py +++ b/tests/test_core/test_collector_contract.py @@ -1046,3 +1046,59 @@ def test_a_marker_is_never_filed_under_its_own_pc( assert comp.assetid not in targets, 'marker filed under its own PC' assert any("own asset number" in w for w in r.get_json()['data']['warnings']) + + +# ============================================================================= +# The subordinate-device map: adding a device type is configuration, not code +# ============================================================================= + +def test_a_site_can_add_a_device_type_without_a_code_change( + client, db, collector_key, computer_assettype, operation_0615): + """The point of the map. A site with two marking lasers on one operation + number needs the same treatment part markers got; before this it needed an + edit to the plugin.""" + import json + from shopdb.core.models import Setting, Asset + + db.session.add(Setting( + key='subordinatedevice_site-laser', + value=json.dumps({'assettype': 'machine', 'typename': 'Marking Laser', + 'description': 'Site laser', 'suffix': 'LASER', + 'partof': True, 'label': 'collector:partmarker'}), + category='pctypemapping')) + db.session.commit() + + client.post('/api/collector/computers', + json={'hostname': 'LASERPC1', 'machinenumber': '0615', + 'pctype': 'site-laser'}, + headers={'X-API-Key': collector_key}) + + device = Asset.query.filter_by(assetnumber='LASERPC1-LASER').first() + assert device is not None + assert device.machine.machinetype.machinetype == 'Marking Laser' + + +def test_a_malformed_override_does_not_break_the_collector( + client, db, collector_key, computer_assettype, operation_0615): + """A bad setting must not stop a bay reporting its inventory.""" + from shopdb.core.models import Setting + + db.session.add(Setting(key='subordinatedevice_gea-shopfloor-partmarker', + value='{not json', category='pctypemapping')) + db.session.commit() + + r = client.post('/api/collector/computers', json=_marker('MARKERPC9'), + headers={'X-API-Key': collector_key}) + assert r.status_code == 200, r.get_json() + + +def test_the_metrology_map_still_answers_through_the_shim(app): + """filters.py and the older tests read metrology_tool_for. Unifying the two + maps must not change what it returns.""" + with app.app_context(): + from plugins.computers.pctypemap import metrology_tool_for + assert metrology_tool_for('gea-shopfloor-cmm') == ( + 'CMM', 'Coordinate measuring machine') + # A machine-typed device is not a measuring tool. + assert metrology_tool_for('gea-shopfloor-partmarker') is None + assert metrology_tool_for('gea-shopfloor-collections') is None