role_facts refuses a scale with a numerator or denominator <= 0; a value at a scaled bound is within it, tested

examples/mcp/server.py

@@ -27,7 +27,6 @@
 the device's state after that command. Nothing is held or resent. Needs mcp and pyserial (requirements.txt) and python/onemachine from this checkout
 (ONEMACHINE_PY=<dir> to use another)."""
 import argparse, json, os, re, sys, threading, time
-from decimal import Decimal
 from fractions import Fraction
 
 HERE = os.path.dirname(os.path.abspath(__file__))
@@ -125,7 +124,7 @@ def refuse(self, args):
                 if v > f.high: return None, '%s: %d is above the maximum %d' % (p, v, f.high)
             return v, None
         hi = self.top()                                                               # scaled: the bounds are on the presented value, as in the schema
-        x = Fraction(Decimal(repr(float(v))))                                         # the number as the client wrote it (12.35, not its binary neighbour)
+        x = f.written(v)                                                              # the number as the client wrote it (1.1, not its binary neighbour)
         if x < 0: return None, '%s: %r is below the minimum 0' % (p, v)
         if x > hi: return None, '%s: %r is above the maximum %s' % (p, v, number(hi))
         return f.to_raw(v), None

python/onemachine/machine.py

@@ -262,10 +262,14 @@ def __repr__(self): return 'Facts(%s %s)' % (self.name, self.kind)
     def presented(self, raw):
         """raw -> the presented value, exactly (a Fraction): raw * num / den"""
         return Fraction(int(raw) * self.scale[0], self.scale[1])
+    def written(self, value):
+        """a number as the client wrote it, exactly (a Fraction): the shortest decimal that round-trips the double, so 1.1 is 11/10 and not the double's
+        binary value 1.100000000000000088...; a value is compared with a presented bound (presented(high)) through this, never as a float"""
+        return Fraction(Decimal(repr(float(value))))
     def to_raw(self, value):
-        """presented -> raw: the exact rational of the shortest decimal that round-trips the client's number, times den / num, to the nearest integer,
-        halves away from zero. Integer arithmetic only: no round(), no float multiply."""
-        x = Fraction(Decimal(repr(float(value)))) * self.scale[1] / self.scale[0]
+        """presented -> raw: the number as written, times den / num, to the nearest integer, halves away from zero. Integer arithmetic only: no round(),
+        no float multiply."""
+        x = self.written(value) * self.scale[1] / self.scale[0]
         p, q = abs(x.numerator), x.denominator
         r = (2 * p + q) // (2 * q)
         return -r if x < 0 else r
@@ -287,7 +291,7 @@ def unpack_text(self, array):
 
 def role_facts(m, name):
     """The Facts of role `name` of Machine m, or KindError. Checks the description against the kind: only the lines the kind prints, the fields it reports and
-    commands present with the types it needs, the bound param, distinct values, unique labels, a positive scale for a scaled value, a flag that is unsigned."""
+    commands present with the types it needs, the bound param, distinct values, unique labels, a positive scale, a flag that is unsigned."""
     info = m.roles[name]
     if info.kind not in KINDS: raise KindError('role %s is of kind %r; python/onemachine has facts for %s' % (name, info.kind, ', '.join(sorted(KINDS))))
     k, f = KINDS[info.kind], Facts(name, info.kind)
@@ -331,9 +335,9 @@ def refuse(why): raise KindError('role %s (%s): %s' % (name, info.kind, why))
         for line, d in (('scale', info.scales), ('unit', info.units)):
             if set(d) != {k.report}: refuse('the description needs one `%s` line, for the field %r (it has %s)' % (line, k.report, sorted(d) or 'none'))
         f.scale, f.unit = info.scales[k.report], info.units[k.report]
+        if f.scale[0] <= 0 or f.scale[1] <= 0: refuse('scale %d/%d is not positive' % f.scale)
         if info.kind == 'scaled':
             if k.upper not in info.params: refuse('no param %r in its description, and the %s kind says it bounds %r' % (k.upper, info.kind, k.command))
-            if f.scale[0] <= 0 or f.scale[1] <= 0: refuse('scale %d/%d is not positive' % f.scale)
             hi = info.params[k.upper]
             if not (0 <= hi <= thi): refuse('bound [0, %d] does not fit the %s field %r' % (hi, f.command_type, k.command))
             f.low, f.high = 0, hi

test/role/facts_check.py

@@ -4,7 +4,7 @@
 import os, sys
 sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), '../../python'))
 from fractions import Fraction
-from onemachine import Machine, StreamLink, KindError, LinkError, role_facts, KINDS
+from onemachine import Machine, StreamLink, SchemaError, KindError, LinkError, role_facts, KINDS
 from onemachine.machine import _fnv, OK
 from onemachine.schema import Field, Layer
 
@@ -83,10 +83,39 @@ def after(ls, line, *new): i = ls.index(line); ls[i + 1:i + 1] = list(new)
 refused('an analog with no `scale` line', lambda ls: drop(ls, 'scale vin raw 5 1023'), 'vin', 'scale')
 refused('an analog whose `unit` is for another field', lambda ls: sub(ls, 'unit vin raw V', 'unit vin other V'), 'vin', 'unit')
 refused('a scaled with a scale of 0/10', lambda ls: sub(ls, 'scale duty raw 1 10', 'scale duty raw 0 10'), 'duty', 'not positive')
+refused('an analog with a scale whose numerator is 0 (0/1023)', lambda ls: sub(ls, 'scale vin raw 5 1023', 'scale vin raw 0 1023'), 'vin', 'scale 0/1023', 'not positive')
+refused('an analog with a negative numerator (-5/1023)', lambda ls: sub(ls, 'scale vin raw 5 1023', 'scale vin raw -5 1023'), 'vin', 'scale -5/1023', 'not positive')
+refused('an analog with a negative denominator (5/-1023)', lambda ls: sub(ls, 'scale vin raw 5 1023', 'scale vin raw 5 -1023'), 'vin', 'scale 5/-1023', 'not positive')
+refused('a scaled with a negative numerator (-1/10)', lambda ls: sub(ls, 'scale duty raw 1 10', 'scale duty raw -1 10'), 'duty', 'scale -1/10', 'not positive')
+refused('a scaled with a negative denominator (1/-10)', lambda ls: sub(ls, 'scale duty raw 1 10', 'scale duty raw 1 -10'), 'duty', 'scale 1/-10', 'not positive')
+try: Machine(Fake(rehash([('scale vin raw 5 0' if l == 'scale vin raw 5 1023' else l) for l in lines]))); check('refused: a scale with denominator 0, by the description itself', False)
+except SchemaError as e: check('refused: a scale with denominator 0, by the description itself -> %s' % e, 'denominator 0' in str(e))
 refused('a scaled with no `max` param', lambda ls: drop(ls, 'param duty max 1000'), 'duty', "'max'")
 refused('a scaled whose max does not fit its field', lambda ls: sub(ls, 'param duty max 1000', 'param duty max 70000'), 'duty', '70000')
 check('the machine as it is is accepted by every refusal harness', all(role_facts(Machine(Fake(rehash(list(lines)))), n) for n in m.roles))
 
+# ---- a value exactly at the presented bound of a scaled role is within it, and a float compared as its binary value is not: 1.1 as a double is above 11/10
+import importlib.util, math
+spec = importlib.util.spec_from_file_location('mcp_server', os.path.join(os.path.dirname(os.path.abspath(__file__)), '../../examples/mcp/server.py'))
+srv = importlib.util.module_from_spec(spec); spec.loader.exec_module(srv)           # examples/mcp's server: the consumer that compares a value with the bound
+def edited(edit):
+    ls = list(lines); edit(ls); return Machine(Fake(rehash(ls)))
+for max_raw, bound, binary in ((11, '1.1', 'above'), (4, '0.4', 'above'), (9, '0.9', 'above'), (3, '0.3', 'below')):            # duty's scale is 1/10: the bound is max/10
+    mm = edited(lambda ls: sub(ls, 'param duty max 1000', 'param duty max %d' % max_raw))
+    f, x = role_facts(mm, 'duty'), float(bound)
+    top = Fraction(max_raw, 10)
+    check('bound %s: presented(high) is exactly %s' % (bound, top), f.presented(f.high) == top)
+    check('bound %s: the double %r is %s the bound in binary (%s)' % (bound, x, binary, 'the comparison that must not be used' if binary == 'above' else 'a control: both comparisons agree'),
+          (Fraction(x) > top) == (binary == 'above') and (Fraction(x) < top) == (binary == 'below'))
+    check('bound %s: written(%r) is the bound, so the value is within it' % (bound, x), f.written(x) == top and f.written(x) <= f.presented(f.high))
+    up = math.nextafter(x, 10)
+    check('bound %s: the next double above (%r) is above the bound' % (bound, up), f.written(up) > f.presented(f.high))
+    role = {r.name: r for r in srv.model(mm)}['duty']
+    raw, err = role.refuse({'value': x})
+    check('examples/mcp, bound %s: duty_set %r is accepted, raw %d' % (bound, x, max_raw), err is None and raw == max_raw, (raw, err))
+    raw, err = role.refuse({'value': up})
+    check('examples/mcp, bound %s: duty_set %r is refused naming the maximum' % (bound, up), raw is None and 'above the maximum %s' % bound in err, (raw, err))
+
 # ---- the field-shape refusals: stub schemas with one field changed
 class S:
     def __init__(self, **layers): self.l = layers