Fix batch_read_vsfrs, and use it to implement alarm level set/get #48
Conversation
The read request should be encoded as int(n)+list[int(vsfr]); telling
the device first how many VSFRs you wish to read, followed by their IDs.
Incorrect encoding will crash your device.
Then, depending on which VSFRs you read, you might want to decode them
as something other than ints. By way of example, you could retrieve the
calibration coefficients by hand with
```
resp = rc103.batch_read_vsfrs(
[VSFR.CHN_TO_keV_A0, VSFR.CHN_TO_keV_A1, VSFR.CHN_TO_keV_A2],
"<4x3f")
```
Inspecting the result from `execute()`, I get back the following 16 byte
response: '07000000f5dbc4c084f51f40bc5ff339'. Without providing a format
that would unhelpfully decode to [7, 3234126837, 1075836292, 972251068]
By providing the format string `<4x3f`, that decodes to
(-6.15185022354126, 2.4993600845336914, 0.00046419899445027113) which
are in fact the calibration coefficients for my device. I get the same
values from the `energy_calib()` function, as well as from querying the
VSFRs individually.
|
This will be used to implement the ability to query and set alarms individually or in a batch. Let me know if you want smaller PRs that do one thing each, or if you want a single PR that just adds this all. |
|
May as well include a consumer of batch_read_vfsrs, specifically the ability to set/get alarm limits. I didn't bother to create a generic `batch_write_vsfrs. I'm guessing that there are few situations aside from setting up alarm thresholds where you'd want to change a whole bunch of things on your device all at once. Maybe the display settings, but I'm either connected over bluetooth and looking at the app, or connected over usb and logging to some custom tooling. |
ckuethe
changed the title
radiacode/radiacode.py
Outdated
| if unpack_format: | ||
| ret = r.unpack(unpack_format) | ||
| else: | ||
| ret = [r.unpack('<I')[0] for _ in range(len(vsfr_ids) + 1)] |
There was a problem hiding this comment.
Why +1 and 'skip first 4 bytes' in '<4x3f to skip the first 4 bytes,`?
What's inside those 4 bytes?
There was a problem hiding this comment.
Even if it's unknown at the moment, I think it's better to explicitly read it and ignore it (with a comment in the code), and not take it into account in unpack_format / return array values
There was a problem hiding this comment.
the first 4 bytes are a bitfield that indicates whether the requested VSFR is valid. in the example, I requested 3 VSRFs (the ones containing the energy calibration), and those are valid, so 3 bits are set.
the point of unpack_format is to allow users to build their own queries in whatever way they want. <4x3f was simply an example; <I3f or <Ifff are just as valid.
There was a problem hiding this comment.
In that case, maybe it's better to unpack only 4 bytes in batch_read_vsfrs, parse it (to list or tuple of bools) and return it with raw response - if we can't distinguish replies to different requests of batch in this function, then I think it's better to construct and parse data on the caller's side.
So, my proposal is:
def batch_read_vsfrs(self, vsfr_ids: list[VSFR]) -> tuple[list[bool], BytesBuffer]
|
I'd need to do some experiments to see what happens if I I kind of did that with |
|
For first 4bytes as |
|
I'm glad my interpretation of the first 32 bits was correct. In my head maybe return type is something like a list of tuples or dataclasses: You asked to read 5 VSFRs, you get back 5 elements, in the same order and same identifier as you asked for. And you get an indicator of whether or not it's data or garbage. actually, value=None would be sufficient to indicate no valid data for that VSFR. you asked for a thing... I got nothin'. |
Callers of batch_read_vsfrs should know the data type of each requested VSFR, and should therefore be able to specify a correct format string. Using an inappropriate format string will raise an error. Require that vsfr_ids is a list of VSFR dataclasses. This prevents the use of arbitrary integers as a VSFR to read. Using other data types will raise an error. Add a check for valid reads. The first item returned by the device is a bitfield indicating which VSFRs were read successfully. Use this to check that batch read operation was completely successful. As only the defined VSFRs are permitted, and those were retrieved from the device, there should be no reason for any read to fail as it might by retrieving an arbitrary location (eg. 0xABAD1DEA). If such a failure is detected, an execption is raised.
ckuethe
force-pushed
the
fix_batch_vsfr_read
branch
from
c681e03 to
c85131b
Compare
|
new version - batch read now requires a format string, which should be easy to construct. it only accept a list of defined VSFRs so you can't query an invalid VSFR, and successful read of all requested VSFRs is checked. |
|
I don't like However, we can easily prepare for that possibility, and the API will be even clearer, something like: def batch_read_vsfrs(self, vsfr_ids: list[tuple[VSFR, str]]) -> list[Optional[Any]]Your proposal using tuples or dataclasses is even better in terms of clarity, though it might be slightly overkill in this context - it's up to you. |
|
I was vaguely thinking about adding an unpack format member to the VSFR definition. Then you wouldn't need to pass in a format specifier, you'd just ask for whatever VSFRs you wanted, and you'd get back a list of decoded values, or as close to decoded as we can figure. But that seemed like too much overengineering without a demonstrated need for it. None of the VSFRs return more than 4 bytes of data when queried. Some appear to only have 1 meaningful byte, though they can be decoded as an int. If you look at the output of the |
Test Scriptimport radiacode
from io import StringIO
import configobj
rc103 = radiacode.RadiaCode()
sio = StringIO(rc103.commands())
commz = configobj.ConfigObj(sio)
for k,v in commz.items():
v.pop('Virtual', None) # they're all "virtual"...
print(f"{k} -> {int(v['Addr'], 16)} : {v}")OutputBut even the Size:1 items are 4 bytes on the wire. |
|
I'm not 100% in agreement with you on this, but I found an assert related to the |
2 participants
The read request should be encoded as int(n)+list[int(vsfr]); telling the device first how many VSFRs you wish to read, followed by their IDs. Incorrect encoding will crash your device.
Then, depending on which VSFRs you read, you might want to decode them as something other than ints. By way of example, you could retrieve the calibration coefficients by hand with
Inspecting the result from
execute(), I get back the following 16 byte response: '07000000f5dbc4c084f51f40bc5ff339'. Without providing a format that would unhelpfully decode to [7, 3234126837, 1075836292, 972251068]By providing the format string
<4x3f, that decodes to (-6.15185022354126, 2.4993600845336914, 0.00046419899445027113) which are in fact the calibration coefficients for my device. I get the same values from theenergy_calib()function, as well as from querying the VSFRs individually.