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_<pctype> 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.
This commit is contained in:
cproudlock
15
CHANGELOG.md
15
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_<pctype>` 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,
|
||||
|
||||
@@ -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_<pctype> 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_<pctype> 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():
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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'))
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user