Fixing slight bug

[evalott100]

Closes #54

There is a slight bug in the current code, sometimes the enum values will come in as strings and sometimes they will come in as integers (corresponding to the index in the field_details.labels).

This adjusts the code to work for either case.

[codecov[bot]]

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (7077b19) 90.72% compared to head (4006dc8) 86.20%. Report is 15 commits behind head on main.

:exclamation: Current head 4006dc8 differs from pull request most recent head 95f1ac5. Consider uploading reports for the commit 95f1ac5 to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #53 +/- ## ========================================== - Coverage 90.72% 86.20% -4.52% ========================================== Files 7 7 Lines 1110 1131 +21 Branches 173 185 +12 ========================================== - Hits 1007 975 -32 - Misses 73 121 +48 - Partials 30 35 +5 ``` | [Files](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks) | Coverage Δ | | |---|---|---| | [src/pandablocks\_ioc/\_hdf\_ioc.py](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks#diff-c3JjL3BhbmRhYmxvY2tzX2lvYy9faGRmX2lvYy5weQ==) | `94.92% <100.00%> (+0.11%)` | :arrow_up: | | [src/pandablocks\_ioc/\_tables.py](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks#diff-c3JjL3BhbmRhYmxvY2tzX2lvYy9fdGFibGVzLnB5) | `78.12% <92.85%> (-17.51%)` | :arrow_down: | | [src/pandablocks\_ioc/\_pvi.py](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks#diff-c3JjL3BhbmRhYmxvY2tzX2lvYy9fcHZpLnB5) | `97.02% <89.47%> (-1.90%)` | :arrow_down: | | [src/pandablocks\_ioc/ioc.py](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks#diff-c3JjL3BhbmRhYmxvY2tzX2lvYy9pb2MucHk=) | `88.30% <40.00%> (-0.48%)` | :arrow_down: | | [src/pandablocks\_ioc/\_\_main\_\_.py](https://app.codecov.io/gh/PandABlocks/PandABlocks-ioc/pull/53?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PandABlocks#diff-c3JjL3BhbmRhYmxvY2tzX2lvYy9fX21haW5fXy5weQ==) | `0.00% <0.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[AlexanderWells-diamond]

When does this bug occur? From the looks of it, this code is only called during construction of a table after receiving all the data from PandA. And PandA will only ever send data in one format (the string? I forget). So I'd like to understand exactly where/when the different form of data is arriving in this function, to ensure we're fixing the bug in the right place.

[evalott100]

When does this bug occur? From the looks of it, this code is only called during construction of a table after receiving all the data from PandA. And PandA will only ever send data in one format (the string? I forget). So I'd like to understand exactly where/when the different form of data is arriving in this function, to ensure we're fixing the bug in the right place.

The problem occurs in the test pana (in the lab). During initialisation we have:

################### PRINTED VALUES
INITIAL_VALUE 0
INITIAL_VALUE_TYPE <class 'int'>
LABELS ['Immediate', 'BITA=0', 'BITA=1', 'BITB=0', 'BITB=1', 'BITC=0', 'BITC=1', 'POSA>=POSITION', 'POSA<=POSITION', 
'POSB>=POSITION', 'POSB<=POSITION', 'POSC>=POSITION', 'POSC<=POSITION']

################## ERROR MESSAGE
ERROR:Exception while initializing softioc
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/ioc.py", line 145, in create_softioc
    ).result()
  File "/dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.10/lib/python3.10/concurrent/futures/_base.py", line 446, in result
    return self.__get_result()
  File "/dls_sw/apps/python/miniforge/4.10.0-0/envs/python3.10/lib/python3.10/concurrent/futures/_base.py", line 391, in __get_result
    raise self._exception
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/ioc.py", line 99, in _create_softioc
    (all_records, all_values_dict, block_info_dict) = await create_records(
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/ioc.py", line 1818, in create_records
    records = record_factory.create_record(
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/ioc.py", line 1607, in create_record
    return self._make_table(record_name, field_info, list_vals)
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/ioc.py", line 1114, in _make_table
    table_updater = TableUpdater(
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 390, in __init__
    initial_value=field_details.labels.index(initial_value),
ValueError: 0 is not in list

[AlexanderWells-diamond]

I can't say I understand this. This code has previously been used to create an IOC from the PandA in the lab.

It looks like this code changed in cb6b6e9abd584edaaa2d56c3682ed36ab86956e1, so I would suspect that that commit is where the difference has come from. I haven't looked deeply into it, but I suspect that commit changes initial_value to be a string rather than an integer by this point.

Ultimately, this code should only be receiving one type of data, not two, so handling two is incorrect.

[evalott100]

Putting it back to what it was:

                scalar_record = builder.mbbIn(
                    scalar_record_name,
                    *field_details.labels,
                    initial_value=initial_value,
                    DESC=scalar_record_desc,
                )

we get an error on the following field

FIELD NAME TRIGGER
INIIAL VALUE Immediate
INIIAL VALUE TYPE <class 'str'>
FIELD LABELS ['Immediate', 'BITA=0', 'BITA=1', 'BITB=0', 'BITB=1', 'BITC=0', 'BITC=1', 'POSA>=POSITION', 'POSA<=POSITION', 'POSB>=POSITION', 'POSB<=POSITION', 'POSC>=POSITION', 'POSC<=POSITION']

If we look up on index without checking for string initial value:

                    initial_value=field_details.labels.index(initial_value),

we get an error for

FIELD NAME TRIGGER
INIIAL VALUE 0
INIIAL VALUE TYPE <class 'int'>
FIELD LABELS ['Immediate', 'BITA=0', 'BITA=1', 'BITB=0', 'BITB=1', 'BITC=0', 'BITC=1', 'POSA>=POSITION', 'POSA<=POSITION', 'POSB>=POSITION', 'POSB<=POSITION', 'POSC>=POSITION', 'POSC<=POSITION']

This is during the ioc._make_table of sequencer tables. With the old method, SEQ1:TABLE fails because it sends across a string initial value "Immediate" for the enum. With the new method SEQ2:TABLE fails because it sends across an integer value 0 for the enum.

[AlexanderWells-diamond]

Looking at the web UI for SEQ2, it looks like there are no rows at all. So it sounds like the issue here is attempting to create a defined record for a row that doesn't exist. I don't know if I ever tested this.

[evalott100]

Yep I suspect that's it too: SEQ2:TABLE has the enum values for trigger defined, but it sends across 0 since the field doesn't exist at all.

I've changed this into a fix which skips table generation if the table doesn't exist.

[evalott100]

@AlexanderWells-diamond

These tables are now skipped (on the current test panda):

PGEN1:TABLE
PGEN2:TABLE
SEQ2:TABLE

I guess we were making PGEN PVs before with default values, I would say we shouldn't be if blocks aren't defined for them?

[AlexanderWells-diamond]

To note what was discussed offline: We have to create the PVs, otherwise we will be unable to add rows to those tables after the IOC is initialised. There is perhaps a bug in pandablocks-client regarding returning values for tables with no rows - if there is no row our code receives an integer, whereas if there is a row we receive a string. This mismatch should probably be resolved in that repository, rather than working around it here.

We should also add tests regarding adding/removing rows when there are no rows initially defined for a table.

[evalott100]

After talking about it more we agree that the default values should remain on the ioc side: pandalocks_client words_to_table shouldn't assume default values 0 but accept that values are None.

The ioc will continue to assume default values of 0 for undefined tables, but use field_labels[0] for the enum values.

TESTING

For the test panda where SEQ1 exists and SEQ2 doesn't we have:

Default values continue to be 0:

caget PANDA:SEQ1:TABLE:OUTB1 -> PANDA:SEQ1:TABLE:OUTB1 16384 0 0 0 0 0 0 ...
caget PANDA:SEQ2:TABLE:OUTB1 -> PANDA:SEQ2:TABLE:OUTB1 16384 0 0 0 0 0 0 ...

Enum values default to the 0th label:

caget PANDA:SEQ1:TABLE:TRIGGER -> 16384 Immediate
caget PANDA:SEQ2:TABLE:TRIGGER -> 16384 Immediate

For SEQ2, the panda sends across no data, field_data[field_name] == [] here:

https://github.com/PandABlocks/PandABlocks-ioc/blob/35360f4e29a32bf7cdc54e9d1d210c52b3d3f729/src/pandablocks_ioc/_tables.py#L357

[evalott100]

@AlexanderWells-diamond

I've moved the test to update_table on a table previously blank table ([]) to a different branch. I'm not convinced the way labels are handled in that function is correct (at least with a newer method of dealing with enums). In the interest of getting a fix for this out quickly I suggest we merge this first.

[evalott100]

It isn't actually introducing those commits, or changing those files. Must have messed up when rebasing and moving commits. I'll open another PR.