Write the float32 unimplemented value as its bit pattern

[dersecure]

SUNS_UNIMPL_FLOAT32 is 0x7fc00000 — a bit pattern, the canonical quiet NaN. But create_unimpl_value hands it to f32_to_data, which packs it as a quantity:

>>> import sunspec2.mb as mb
>>> mb.create_unimpl_value('float32', len=4).hex()
'4eff8000'

0x4EFF8000 is struct.pack('>f', 2143289344.0). So a point recorded as unimplemented is serialised as the perfectly ordinary number 2143289344.0, which a reader cannot distinguish from a measurement.

Consequence

The bytes on the wire are wrong for any reader that decodes them per the spec. Model 113 is a good example, since its temperatures are float32 and are frequently unimplemented:

import sunspec2.file.client as fc

d = fc.FileClientDevice('device.json')   # TmpSnk recorded as null
d.scan()
m = next(x for x in d.model_list if x.model_id == 113)

m.get_dict()['TmpSnk']                   # None, correct
mb.data_to_f32(m.get_mb()[...])          # 2143289344.0, not None

I met this comparing an independent decoder against this library across a set of captured devices: every unimplemented float32 came back as 2143289344.0 from the serialised registers while get_dict() reported None for the same point.

The reader was always right

data_to_f32 already maps the NaN pattern to None:

def data_to_f32(data):
    f = struct.unpack('>f', data[:4])
    if str(f[0]) != str(float('nan')):
        return f[0]

So this is the writer disagreeing with the reader, not an ambiguity about what the sentinel means. Packing the bit pattern lets the two meet.

Consistency

Every other type in create_unimpl_value already emits its sentinel's bits — int16 → 80 00, uint16 → ff ff, acc32 → 00 00 00 00, and so on. float32 is the only one that encodes the sentinel as a value first. The change brings it into line:

+    elif vtype == mdef.TYPE_FLOAT32:
+        return struct.pack('>I', SUNS_UNIMPL_FLOAT32)

A changed assertion

test_create_unimpl_value pinned the old bytes:

-    assert mb.create_unimpl_value('float32') == b'N\xff\x80\x00'
+    assert mb.create_unimpl_value('float32') == b'\x7f\xc0\x00\x00'

Flagging that explicitly since it is an existing test changing rather than a new one passing. Two tests are added alongside it: that the bytes are the sentinel pattern and specifically not the quantity, and that an unimplemented float32 now round-trips back to None. Both fail without the change.

Test results

	result
master	174 passed
this branch	176 passed

Three test modules are excluded from both runs (test_modbus_client.py, test_modbus_modbus.py, test_tls_client.py). That is a fault in my environment, not this repository: I have an unrelated PyPI package named serial shadowing pyserial, and it pulls in future, which does import imp — removed in Python 3.12 — so those three fail at collection.

Related, not changed here

create_unimpl_value('float64') raises struct.error: SUNS_UNIMPL_FLOAT64 exists but float64 has no entry in unimpl_value, so None reaches f64_to_data. No bundled model uses float64, so I have left it alone rather than widen this change, but it looks like the same root cause and you may want it.

---

Maintainer note (edited into this description): the sentence above originally read "a device file with an absent float32 no longer round-trips". That claim is too broad, so it has been narrowed to the encoding. The library's own round-trip was never broken: data_to_f32(b'N\xff\x80\x00') returns 2143289344.0, which is exactly SUNS_UNIMPL_FLOAT32, so is_impl_float32 rejects it and set_mb sets the point to None. Checked at the device level on both sides of the change:

master:      unimpl float32 bytes: 4eff8000 -> data_to_f32: 2143289344.0 -> after set_mb, value = None
this branch: unimpl float32 bytes: 7fc00000 -> data_to_f32: None         -> after set_mb, value = None

The defect is confined to the encoding, which is still worth fixing for exactly the reason the description opens with: an external, spec-conformant reader sees a measurement where there is none.

The same mechanism means read compatibility is preserved by this change, which is the question a reviewer is most likely to ask: register images written by earlier versions still decode as unimplemented.

Revert this block if you would rather state it differently.

[Kudrat9]

Verified by cloning at 03aaa7f with the sunspec2/models submodule initialized and running CI-style pytest . per branch: master 234 passed, this branch 236, zero failures. Reverting the change while keeping the tests fails all three affected tests, including the amended test_create_unimpl_value.

The encoding is correct: 4 bytes, matching point_type_info[float32].len of 2 registers, and data_to_f32 maps the result to None. Read-side compatibility is preserved, which is worth stating explicitly in the description since it is the first question a reviewer will have: register images written by earlier versions still decode as unimplemented, because data_to_f32(b'N\xff\x80\x00') returns 2143289344.0 and is_impl_float32 rejects that value.

Should fix:

• The description overstates the defect, and it will become the merge commit message. "A device file with an absent float32 no longer round-trips" does not hold at the library level. The old encoding decodes to exactly 2143289344.0, which equals SUNS_UNIMPL_FLOAT32, so is_impl_float32 returns False and set_mb sets the point to None. Checked at the device level on both master and this change:

  master:      unimpl float32 bytes: 4eff8000 -> data_to_f32: 2143289344.0 -> after set_mb, value = None
  this branch: unimpl float32 bytes: 7fc00000 -> data_to_f32: None         -> after set_mb, value = None
  

  The genuine defect is narrower and still worth fixing: the bytes on the wire are wrong for any spec-conformant external reader, which is exactly the case the description opens with. Suggest reframing around on-wire conformance and dropping the round-trip claim.

Questions:

• is_impl_float32 compares the decoded float against the integer SUNS_UNIMPL_FLOAT32 (2143289344) rather than testing for NaN. After this change nothing reaches it with that value by way of data_to_f32, so the comparison becomes vestigial. Is folding a NaN check into this PR preferable, or is leaving it for a separate change intended?

[Kudrat9]

No code change requested here, so nothing has been pushed to the branch.

The description has been edited (maintainer edit): the round-trip sentence is narrowed to the encoding, and a marked note at the bottom records why, along with the read-compatibility point. Revert that block if you would rather phrase it yourself.

The is_impl_float32 item is left as an open question rather than implemented, since switching it to a NaN test is a behavior change beyond this PR's scope and is yours to call.