Changed enum values back to uint8 before updating the table #58

evalott100 · 2023-10-06T08:36:51Z

Closes #55

Added a test to add to the panda table after initially leaving it blank

update_table will currently not work for enum values in the table since they're List[str] and have to be converted back to their integer index values.

Added a test to update_table for a table which is [] on ioc start up, then is added to on the panda side.

evalott100 · 2023-10-10T08:50:42Z

Requires release of PandABlocks/PandABlocks-client#58 before merging, tested against https://github.com/evalott100/PandABlocks-client/tree/make_conversion_to_string_enums_optional

Added a test to add to the panda table after initially leaving it blank

codecov · 2023-10-11T13:11:34Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (11cad5d) 86.20% compared to head (952efe5) 86.26%.

❗ Current head 952efe5 differs from pull request most recent head 76cd50e. Consider uploading reports for the commit 76cd50e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
+ Coverage   86.20%   86.26%   +0.06%     
==========================================
  Files           7        7              
  Lines        1131     1136       +5     
  Branches      185      171      -14     
==========================================
+ Hits          975      980       +5     
  Misses        121      121              
  Partials       35       35

Files	Coverage Δ
src/pandablocks_ioc/_tables.py	`78.54% <100.00%> (+0.41%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AlexanderWells-diamond

I think we need a couple more tests, especially as the one line of actual code change doesn't have a test covering it.

src/pandablocks_ioc/_tables.py

tests/fixtures/panda_data.py

tests/test_ioc_system.py

evalott100 · 2023-10-13T08:06:04Z

TESTING

EDIT: will try this again later today and better track what values the PVs are expecting

@AlexanderWells-diamond

On an old commit https://github.com/PandABlocks/PandABlocks-ioc/tree/a8586b9fde836a8a8eceda770c935f39af57f98c

SEQ2 Table not defined yet

pandablocks-ioc softioc [test panda ip] PANDA outputted_screens

PANDA:SEQ2:TABLE:MODE          VIEW 
caput PANDA:SEQ2:TABLE:MODE 1
  Old : PANDA:SEQ2:TABLE:MODE          VIEW
  New : PANDA:SEQ2:TABLE:MODE          EDIT
caget PANDA:SEQ2:TABLE:TRIGGER
  PANDA:SEQ2:TABLE:TRIGGER 16384 0 0 0 0
caput -a PANDA:SEQ2:TABLE:TRIGGER 0 1 0
  Old : PANDA:SEQ2:TABLE:TRIGGER 16384 0 0 0
  New : PANDA:SEQ2:TABLE:TRIGGER 16384 1 0 0
caget PANDA:SEQ2:TABLE:TRIGGER
  PANDA:SEQ2:TABLE:TRIGGER 16384 1 0 0
caput PANDA:SEQ2:TABLE:MODE 2
  Old : PANDA:SEQ2:TABLE:MODE          EDIT
  New : PANDA:SEQ2:TABLE:MODE          SUBMIT

The ioc outputted

>>> WARNING:Value 0.0 of record SEQ2:TABLE:TRIGGER is not a recognised value.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 666, in _update_scalar
    scalar_val = labels.index(scalar_val)
ValueError: 0.0 is not in list
WARNING:Value 1.0 of record SEQ2:TABLE:TRIGGER is not a recognised value.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 666, in _update_scalar
    scalar_val = labels.index(scalar_val)
ValueError: 1.0 is not in list
INFO:Sending table data for SEQ2:TABLE to PandA
ERROR:Unable to Put record SEQ2:TABLE, value [],to PandA. Rolling back to last value from PandA.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 533, in update_mode
    packed_data = TablePacking.pack(
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 171, in pack
    curr_val = [field_details.labels.index(x) for x in curr_val]
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 171, in <listcomp>
    curr_val = [field_details.labels.index(x) for x in curr_val]
ValueError: 1.0 is not in list

SEQ1 Table set up in web ui

pandablocks-ioc softioc [test panda ip] PANDA outputted_screens

PANDA:SEQ1:TABLE:MODE          VIEW 
caput PANDA:SEQ1:TABLE:MODE 1
  Old : PANDA:SEQ1:TABLE:MODE          VIEW
  New : PANDA:SEQ1:TABLE:MODE          EDIT
caget PANDA:SEQ1:TABLE:TRIGGER
  PANDA:SEQ1:TABLE:TRIGGER 16384 BITA=0 POSB>=POSITION BITA=1
caput -a PANDA:SEQ1:TABLE:TRIGGER 0 1 0
  Old : PANDA:SEQ1:TABLE:TRIGGER 16384 BITA=0 POSB>=POSITION BITA=1 
  New : PANDA:SEQ1:TABLE:TRIGGER 16384 1 0
caput PANDA:SEQ1:TABLE:MODE 2
Old : PANDA:SEQ1:TABLE:MODE          EDIT
New : PANDA:SEQ1:TABLE:MODE          SUBMIT

The ioc outputted

WARNING:Value 1 of record SEQ1:TABLE:TRIGGER is not a recognised value.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 666, in _update_scalar
    scalar_val = labels.index(scalar_val)
ValueError: '1' is not in list
INFO:Sending table data for SEQ1:TABLE to PandA
ERROR:Unable to Put record SEQ1:TABLE, value [],to PandA. Rolling back to last value from PandA.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 533, in update_mode
    packed_data = TablePacking.pack(
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 171, in pack
    curr_val = [field_details.labels.index(x) for x in curr_val]
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 171, in <listcomp>
    curr_val = [field_details.labels.index(x) for x in curr_val]
ValueError: '1' is not in list

None of the trigger values were changed on the webui.

On this branch (`ensure_adding_to_a_non_defined_table_works`)

SEQ2 Table not defined yet

pandablocks-ioc softioc [test panda ip] PANDA --screens-dir=outputted_screens --clear-bobfiles

PANDA:SEQ2:TABLE:MODE          VIEW 
caput PANDA:SEQ2:TABLE:MODE 1
  Old : PANDA:SEQ2:TABLE:MODE          VIEW
  New : PANDA:SEQ2:TABLE:MODE          EDIT
caget PANDA:SEQ2:TABLE:TRIGGER
  PANDA:SEQ2:TABLE:TRIGGER 16384 0 0 0 0
caput -a PANDA:SEQ2:TABLE:TRIGGER 0 1 0
  Old : PANDA:SEQ2:TABLE:TRIGGER 16384 0 0 0
  New : PANDA:SEQ2:TABLE:TRIGGER 16384 1 0 0
caget PANDA:SEQ2:TABLE:TRIGGER
  PANDA:SEQ2:TABLE:TRIGGER 16384 1 0 0
caput PANDA:SEQ2:TABLE:MODE 2
  Old : PANDA:SEQ2:TABLE:MODE          EDIT
  New : PANDA:SEQ2:TABLE:MODE          SUBMIT

The ioc outputted

>>> INFO:Sending table data for SEQ2:TABLE to PandA
ERROR:Unable to Put record SEQ2:TABLE, value [],to PandA. Rolling back to last value from PandA.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 555, in update_mode
    packed_data = table_to_words(table, self.field_info)
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/.venv/lib/python3.10/site-packages/pandablocks/utils.py", line 111, in table_to_words
    assert len(packed) == len(column), (
AssertionError: Table record TRIGGER has mismatched length compared to other records, cannot pack data

SEQ1 Table set up in web ui

PANDA:SEQ1:TABLE:MODE          VIEW
caput PANDA:SEQ1:TABLE:MODE 1
  Old : PANDA:SEQ1:TABLE:MODE          VIEW
  New : PANDA:SEQ1:TABLE:MODE          EDIT
caget PANDA:SEQ1:TABLE:TRIGGER
  PANDA:SEQ1:TABLE:TRIGGER 16384 1 9 2
caput -a PANDA:SEQ1:TABLE:TRIGGER 0 1 0
  Old : PANDA:SEQ1:TABLE:TRIGGER 16384 1 9 2 0
  New : PANDA:SEQ1:TABLE:TRIGGER 16384 1 0 0 0
caput PANDA:SEQ1:TABLE:MODE 2
  Old : PANDA:SEQ1:TABLE:MODE          EDIT
  New : PANDA:SEQ1:TABLE:MODE          SUBMIT

The ioc outputted

>>> INFO:Sending table data for SEQ1:TABLE to PandA
ERROR:Unable to Put record SEQ1:TABLE, value [],to PandA. Rolling back to last value from PandA.
Traceback (most recent call last):
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/src/pandablocks_ioc/_tables.py", line 555, in update_mode
    packed_data = table_to_words(table, self.field_info)
  File "/scratch/twj43146/Programming/PandABlocks-ioc-fork/.venv/lib/python3.10/site-packages/pandablocks/utils.py", line 111, in table_to_words
    assert len(packed) == len(column), (
AssertionError: Table record TRIGGER has mismatched length compared to other records, cannot pack data

AlexanderWells-diamond

Thank you for testing those situations. To me it looks like this branch is now behaving as expected: Attempting to add a new row by only extending 1 of the waveforms triggers an error.

Do you think this PR is a suitable place to add a test that properly adds a row via EPICS, i.e. does a caput to each and every column in a table?

Otherwise, please fix the CI linting errors and then two minor cleanup comments, then this is good to go.

tests/fixtures/mocked_panda.py

tests/test_ioc_system.py

evalott100 · 2023-10-13T14:34:37Z

Putting it back to how it was before all this,

caput -a PANDA:SEQ1:TABLE:TRIGGER 3 Immediate Immediate "BITA=0" works.

I've made a new branch on the client which allows for packing of empty tables (it gives things default values if columns are missing).

evalott100 · 2023-10-16T09:19:43Z

PandABlocks/PandABlocks-client#59

evalott100 · 2023-10-16T09:25:51Z

Thank you for testing those situations. To me it looks like this branch is now behaving as expected: Attempting to add a new row by only extending 1 of the waveforms triggers an error.

Do you think this PR is a suitable place to add a test that properly adds a row via EPICS, i.e. does a caput to each and every column in a table?

Otherwise, please fix the CI linting errors and then two minor cleanup comments, then this is good to go.

@AlexanderWells-diamond I've now put this back to how it was, before any of these changes. Whenever PVs are changed internally we push the enums as strings.

… before submitting it to the panda

evalott100 requested a review from AlexanderWells-diamond October 6, 2023 08:36

evalott100 self-assigned this Oct 6, 2023

evalott100 force-pushed the ensure_adding_to_a_non_defined_table_works branch 2 times, most recently from 52bd496 to 2e7892f Compare October 10, 2023 08:48

Changed enum values back to uint8 before updating the table

f02b869

Added a test to add to the panda table after initially leaving it blank

evalott100 force-pushed the ensure_adding_to_a_non_defined_table_works branch from 2e7892f to f02b869 Compare October 10, 2023 12:16

AlexanderWells-diamond requested changes Oct 11, 2023

View reviewed changes

src/pandablocks_ioc/_tables.py Outdated Show resolved Hide resolved

tests/fixtures/panda_data.py Outdated Show resolved Hide resolved

tests/test_ioc_system.py Outdated Show resolved Hide resolved

Improved tests

8df422a

evalott100 requested a review from AlexanderWells-diamond October 12, 2023 13:30

AlexanderWells-diamond approved these changes Oct 13, 2023

View reviewed changes

tests/fixtures/mocked_panda.py Outdated Show resolved Hide resolved

tests/test_ioc_system.py Outdated Show resolved Hide resolved

evalott100 added 2 commits October 16, 2023 10:23

Using string representations of enums again

4400d9d

Changed client branch to test ci working

22df0d6

evalott100 force-pushed the ensure_adding_to_a_non_defined_table_works branch from ee7ec2c to 22df0d6 Compare October 16, 2023 09:23

Ran isort

952efe5

evalott100 force-pushed the ensure_adding_to_a_non_defined_table_works branch from 894864e to 952efe5 Compare October 16, 2023 09:57

Changed a test to explicitly define all values on the undefined table…

76cd50e

… before submitting it to the panda

evalott100 force-pushed the ensure_adding_to_a_non_defined_table_works branch from 1a3877b to 76cd50e Compare October 16, 2023 15:15

evalott100 merged commit ea8ca01 into PandABlocks:main Oct 16, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changed enum values back to uint8 before updating the table #58

Changed enum values back to uint8 before updating the table #58

evalott100 commented Oct 6, 2023

evalott100 commented Oct 10, 2023 •

edited

Loading

codecov bot commented Oct 11, 2023 •

edited

Loading

AlexanderWells-diamond left a comment

evalott100 commented Oct 13, 2023 •

edited

Loading

AlexanderWells-diamond left a comment •

edited

Loading

evalott100 commented Oct 13, 2023

evalott100 commented Oct 16, 2023

evalott100 commented Oct 16, 2023

Changed enum values back to uint8 before updating the table #58

Changed enum values back to uint8 before updating the table #58

Conversation

evalott100 commented Oct 6, 2023

evalott100 commented Oct 10, 2023 • edited Loading

codecov bot commented Oct 11, 2023 • edited Loading

Codecov Report

AlexanderWells-diamond left a comment

Choose a reason for hiding this comment

evalott100 commented Oct 13, 2023 • edited Loading

TESTING

EDIT: will try this again later today and better track what values the PVs are expecting

On an old commit https://github.com/PandABlocks/PandABlocks-ioc/tree/a8586b9fde836a8a8eceda770c935f39af57f98c

SEQ2 Table not defined yet

SEQ1 Table set up in web ui

On this branch (ensure_adding_to_a_non_defined_table_works)

SEQ2 Table not defined yet

SEQ1 Table set up in web ui

AlexanderWells-diamond left a comment • edited Loading

Choose a reason for hiding this comment

evalott100 commented Oct 13, 2023

evalott100 commented Oct 16, 2023

evalott100 commented Oct 16, 2023

evalott100 commented Oct 10, 2023 •

edited

Loading

codecov bot commented Oct 11, 2023 •

edited

Loading

evalott100 commented Oct 13, 2023 •

edited

Loading

On this branch (`ensure_adding_to_a_non_defined_table_works`)

AlexanderWells-diamond left a comment •

edited

Loading