Write to NTTables

[jjaraalm]

Currently, the table widget does not appear to write to NTTable PVs. Since NTTable is well defined, it would be nice if there was (limited) support for writing to NTTable PVs.

Potential Issues

• It looks like all tables are lists of lists of strings, so only tables of strings should be supported for now. It would be the server's job to validate entries and convert data internally, Phoebus would just pass on whatever raw data is entered.

Suggested Options

• Allow different update modes
  1. Write to PV on focusout. In other words, after each cell is updated, update the PV. In this mode, regular PV updates could continue to occur on all non-focused cells.
  2. Write to PV on blur. Once the user clicks away from the table, the PV is updated. In this mode, regular PV updates should be paused while the table is being edited.
• Configuration option to prevent creation of rows (hide buttons in header)
• Configuration option to prevent creation of columns (hide buttons in header)
• Configuration options to prevent editing column labels (hide buttons in header)

[ttkorhonen]

Has anybody taken a look at this issue? It is still unassigned. The write functionality would be a really highly appreciated feature to have.
I am not sure if all tables are arrays of strings though, at least this is not how we (ESS) foresee the tables to be used.

[shroffk]

We discussed this in today's meetings.. we have 2 AI associated with this request

[ttkorhonen]

All right, that sounds promising, thanks!

[kasemir]

Each table use case seems to basically describe its own little application, requiring specialized behavior.
You can already handle that via a script behind the table where you put that application code. Alternatively, you are adding an open-ended list of properties and options to the table widget to describe all the possible scenarios, which I don't think is practical.

At best you could add "limited" write support, as you suggest, only for string cells.
But when to write? As a control system GUI, we generally require some sort of confirmation.
In a text field, you need to type 'ENTER' to write. Leaving focus/blur/click outside/press Escape/... all restore the original value. I don't think it's a good idea to change this for the table, where you would suddenly write many values without confirmation.

[ttkorhonen]

Well, I can do pvput's to NTTables so I do not immediately see why that would not be doable from a table widget?
I do not fancy a lot having users to write scripts each time they want to use a table widget.
OTOH, I fully agree with requiring "ENTER" or similar to write. Write on focusout...no, please not. Editing a table and then somebody taps your shoulder...

[shroffk]

So I think the agreement was to aim for a "limited" write support... something that covers basic cases and avoids the use of scripts.
This could surely limit the type of write supported to only string cells...

I agree with maintaining the "confirmation" behavior... we can do a write each cell when Enter is pressed.

[kasemir]

we can do a write each cell when Enter is pressed

That's good because it matches the text entry field behavior.
Also means the table widget doesn't need to buffer the 'current' value of all cells, somehow highlight what changed and would be written in case we did write. All that's limited to just one cell.

But I don't think you can write just one cell, so each cell change would write the complete table.
And you have to handle the case where the table keeps changing while you edit.
Lock changes to the complete table until the edited cell is either submitted or restored?
Lock changes just to the edited cell until that cell is submitted or restored?

[shroffk]

Also means the table widget doesn't need to buffer the 'current' value of all cells, somehow highlight what changed and would be written in case we did write. All that's limited to just one cell.

exactly.. going for simple and predictable behaviour here

But I don't think you can write just one cell, so each cell change would write the complete table.

We brought this up for discussion, it turns out that all the use cases from ESS and DLS would be fine with writing the complete table when updating a single cell.

And you have to handle the case where the table keeps changing while you edit.
Lock changes to the complete table until the edited cell is either submitted or restored?
Lock changes just to the edited cell until that cell is submitted or restored?

We can bring up this issue...
I would prefer to lock the entire table, this has been the behaviour in the old cs-studio table widget and is the behaviour of the pvTable applications. Thus locking the entire table would maintain a consistent behaviour.. Also from what I understand the tables which need write support usually consists of configuration (DLS) or setpoint or similar signals which are not actively changing.

[ttkorhonen]

In fact, I would even expect and insist on writing the full table. But after updating each individual cell...? That could be a problem. If you are for instance updating a (breakpoint) conversion table (just imagining here), you want to have the full table completed before committing it.
There may be other solutions to handle this however.

[kasemir]

for instance updating a (breakpoint) conversion table

Right, in that case you probably want to submit a consistent set of values, and assert that the values are increasing along one axis, which can be handled by a simple "values_in_column_1_increase_monotonically" option on the table widget.
That's exactly it, each use case has its own set of concerns. You can implement each use case as a standalone PyQt application, or put the logic into a script behind the table. But I'm afraid that adding all the application logic elements to the table widget will soon get tiresome.

[ttkorhonen]

That was a bit of an imagined example. I would not encourge editing breakpoint tables in a table widget anyway, this would be too risky for a number of reasons. But I still would not back up from the original argument of covering the generic use case a la pvput. No custom logic required.

[jjaraalm]

Yes, there are application specific issues, but I don't think it's hopeless.

For the breakpoint example, it would make more sense for the IOC to validate the table input and reject invalid tables. I think what @ttkorhonen is asking for is just a single option for write on blur (for the table) vs focusout (for the cell) as suggested in the original issue. This should be a simple boolean option for the widget.

[ttkorhonen]

I must add that I have no idea about how the table widget is implemented. If I write a script, would that script be executed after each cell update?

[kasemir]

The script is for example attached to a "Submit" button.
Script then reads from the table and maybe other elements on the display, combines the data into whatever it needs, and sends that to a web servlet or one or more PVs or a file or ... depending on the application.

[ttkorhonen]

Right. Then instead of the script, we could have a piece of Java code that reads the elements from the table and does a pvput. This is more or less all I would ask for. If I need custom logic, I would write a script or do something else.

[kasemir]

My main concern is an explosion of widget properties and logic.
If the table widget has a PV, and it's of type VTable, you can disable the column creation and header renaming, since that information comes from the PV. If the PV is writable, and only contains strings, you can enable editing, with simple write-table-on-ENTER-in-cell behavior. Seems simple enough, so please go ahead and implement it.

[jjaraalm]

That makes sense.

I would love to help, but unfortunately the project I originally needed this for has been put on indefinite hold and I don't really have the bandwidth to take this on currently.

[thomascobb]

After talking to @aawdls I believe the DLS requirement is one that originates from me, so I'll put it here for completeness.

A PandABox is a configurable FPGA based trigger box. One of its functions is a sequencer table, which takes rows of trigger enum, number of repeats, phased times and output selections. It is currently controlled via a web GUI that looks like this:

In the web GUI we don't send the value of the table down to the server until the submit button is pressed, and there is a discard button to revert to the server's copy.

However, sites have requested an EPICS IOC that lets you set every single parameter of a PandA. I've packaged every column of the table in a waveform record, and tied them together into an NTTable in the IOC with QSRV. This is not quite working, but is almost there.

I hoped to be able to attach a phoebus table widget to this NTTable, but haven't tried it out yet, so this ticket is very relevant. A slight issue is that the columns have different datatypes, uint8, uint32, and enum. The first two are expressible in an NTTable, the last is not. I'm going to talk about this in the EPICS core meeting Wednesday next week to work out how this could be expressed in an NTTable, but I see that the phoebus widget already the ability to add choices to a column so maybe this will not be an issue?

[thomascobb]

I'm not at all familiar with Phoebus, but I've had a go at mocking up a table that switches between Displaying and Editing:

In Displaying mode the widget displays updates from the PV. When you click Edit the widget stops updating and becomes editable:

If you click Submit then it writes to the PV and goes back to Displaying. If you click Discard then it just goes back to Displaying.
The script to do this:

from org.csstudio.display.builder.runtime.script import PVUtil, ScriptUtil

mode = PVUtil.getLong(pvs[0])
table = PVUtil.getTable(pvs[1])

if mode == 0:  # Displaying
	widget.setValue(table)
	widget.setPropertyValue("editable", False)
elif mode == 1:  # Editing
	widget.setPropertyValue("editable", True)
elif mode == 2:  # Submit
	pvs[0].write(0)
	pvs[1].write(widget.getValue())

is in the repo along with the QSRV ioc that serves the NTTable: https://github.com/thomascobb/nt_table_test

I can't make a couple of things work:

• Writing a VTable to an NTTable PV fails with:

$ 2021-11-09 17:11:29 WARNING [org.epics.pva] TCP sender /192.168.1.176:60125 request encoding error
java.lang.Exception: Cannot set  value to [[false, Option 1, 3359], [false, Option 2, 1], [false, Option 3, 2], [true, Option 1, 3], [false, Option 2, 4], [false, Option 3, 5], [false, Option 1, 6], [false, Option 2, 7], [false, Option 3, 8], [false, Option 1, 9]]
	at org.epics.pva.data.PVAStructure.setValue(PVAStructure.java:110)
	at org.epics.pva.client.PutRequest.encodeRequest(PutRequest.java:138)
	at org.epics.pva.common.TCPHandler.sender(TCPHandler.java:179)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

• widget.setPropertyValue("editable", True) doesn't seem to make it editable, even though the property sets
• I can only get this to work with NTTables with string columns, even though there seems to be some code written for other types: https://github.com/ControlSystemStudio/phoebus/blob/master/core/pv/src/main/java/org/phoebus/pv/pva/PVAStructureHelper.java#L160

[thomascobb]

I've worked around the last point by converting from number to string in the embedded script. Obviously type conversion in an embedded Jython script is not a long term solution, but it is sufficient to demonstrate the principle.

I've also made a proposal for getting display_t information for each column into NTTable so it doesn't have to be configured in the widget: https://epics.anl.gov/core-talk/2021/msg01390.php

[jjaraalm]

@thomascobb Your use case is almost identical to mine. I was able to write to NTTables by doing the following:

from org.csstudio.display.builder.runtime.script import ScriptUtil
from org.epics.pva.data import PVAStructure, PVAStringArray

table = ScriptUtil.findWidgetByName(widget, "table")
data = table.getValue()

# Modify data ...

pv = ScriptUtil.getPrimaryPV(table)
ncol = len(data[0])
ntrows = [PVAStringArray("column"+str(i), [row[i] for row in data]) for i in range(ncol)]
pv.write(PVAStructure("", "", ntrows))

You can modify the individual column types (e.g., PVAIntArray()) as needed. One downside with this method is that the timestamp is not updated, and I'm not sure how to easily do that without modifying phoebus.

[thomascobb]

@jjaraalm thanks, that looks like just what I need!

[georgweiss]

So based on push from users I started to look into "native" support for writing NTTable from Phoebus. I've come far enough to construct a Java object that - based on my understanding at least - should be good enough. The write operation seems to succeed as the IOC shell echos a message, and a pvmonitor on the PV in question also echoes.

Problem is the actual values in the table are unchanged, kind of a deal breaker here.

To summarize, I have table with two integer array columns. In PVAStructure#setValue() I simply construct two PVAIntArray objects and set the value of the class' field elements (same types) to these new values. Attached screen show shows what it looks like after PVAStructure#setValue().

@kasemir , @shroffk , obviously I'm missing something...

[kasemir]

The above examples all use a string table, while you have integer columns. Maybe try string columns.

[georgweiss]

Not sure I understand why strings should work. After all, we know what underlying types we're dealing with.

[georgweiss]

To clarify, I'm not (yet) implementing write from Table widget. My use case is save&restore.

[georgweiss]

Digging deeper into my failed attempts to actually write data I have identified the reason: the BitSet portion of the put request. Tests performed on the following PV running on a 7.0.7 server:

 epics:nt/NTTable:1.0
        structure record
            structure _options
                uint queueSize
                boolean atomic
        alarm_t alarm
            int severity
            int status
            string message
        structure timeStamp
            long secondsPastEpoch
            int nanoseconds
            int userTag
        string[] labels
        structure value
            int[] A
            int[] B

So when trying to update values in arrays A and B the client currently sets only bit 14 (structure value), but I have verified that it must also set bit 15 and 16. Or even set only bit 15 and 16. Or should the client send two put requests, one for each element?

In PutRequest the field in question is thus correctly identified (as bit 14) but the elements of the structure are not identified and added to the BitSet object.

Maybe I'm not using the API correctly...?

@kasemir, can you please comment?

[kasemir]

The bitfield usage might be the reason. What we've tried so far is reading and sometimes writing the types presented by IOCs, where we don't get any table data. Brings up the question how to duplicate this.

[georgweiss]

Well, with code adding the "missing" indices I am able to replicate the CLI pvput functionality.

[kasemir]

Did you update the existing code so that when it flags the "structure" as changed, it will recurse inside the structure and flag all elements as changed? Or do you just manually flag "A" and "B" to test this specific case?

[georgweiss]

Don't worry, won't push my hack.
If it is a structure then I add the bits for all it's elements (Requires access to the PVAStructure#elements field).

[kasemir]

If it is a structure then I add the bits for all it's elements

If a structure is flagged as changed, does that mean
a) The whole structure has changed. That's why the structure is flagged!
b) Nothing?! Each element that changed needs to be individually flagged?

I don't think the exact behavior is defined anywhere, but you clearly see the latter case.
What type of server is that? Custom pvAccess C++ code? pvapy using pvAccess? Custom PVXS C++ code? p4p using PVXS?

Since we have at least one server that requires each element to be flagged, we can use that as the defining case since it should still "work" for those servers that go by just the overall structure flag, so we won't break anything.

If it is a structure then I add the bits for all it's elements

Feel free to turn that into a PR

[georgweiss]

a) The whole structure has changed

If the structure has two arrays and only one of them changes, is the "whole" structure changed?

b) Each element that changed needs to be individually flagged?

That is what I conclude and that is what pvput does.

What type of server is that?

Vanilla Epics 7.0.7 built on Mac. With pvAccess module.

[kasemir]

How do you create the table? 'group' settings on records? Can you post that?

[georgweiss]

nttable.txt

[georgweiss]

I found a thing a bit concerning on page 12 in https://epics-controls.org/wp-content/uploads/2018/10/pvAccess-Protocol-Specification.pdf:

BitSet does not apply to array elements

But maybe that spec is outdated...

[kasemir]

That refers to array elements. We always flag the whole array as changed, and send the complete array. There is nothing in the protocol to say: For array X, replace element 5 with 47, then set elements 10 to 20 to 0. No support for slices within an array.

[georgweiss]

I see. Apparently "array elements" can be interpreted in multiple ways...

[kasemir]

Yes, it's meant as "BitSet does not apply to individual array elements. One bit in bitset addresses the complete array".

[kasemir]

That group-based table example is good. Include that in a PR which recursively flags the elements of a structure.

[georgweiss]

Kudos to @ttkorhonen for providing me with the table example.

[kasemir]

group-based table example is good.

Well, good as in providing an easy to use example.
I think it shows that the current Q:group syntax is somewhat complicated and I'd be very surprised if that remains THE way to define PVA structures in the IOC.

[ttkorhonen]

I got mentioned :-)
In fact, I got the example from Michael's early demo IOCs, but at some of development stage the original got outdated. I then fixed it so that it works with the current implementation. I also changed it slightly to be easier to relate to...

Michael has improved the group PV documentation in the meantime: https://mdavidsaver.github.io/pvxs/qgroup.html
Earlier, it was a challenge to figure out the syntax, I sometimes had even to read the source code to understand what is happening.

And yes, the definition syntax is somewhat complicated but also powerful. However, if you ask me, it is on the easier side of things to learn related to PVA. But sure, one could probably make it easier. I just do not have any good ideas how.

[georgweiss]

@kasemir, one more question before I proceed.

By default PutRequest will update the value field of a structure. Should update of the labels string array also be supported such that both value and labels are updated in the same put request?

[kasemir]

I'd leave the labels alone. With IOC records that support enumerated values (bo, mbbo) you receive the value with labels. You can write the value, but I don't think you can write the labels.

So change the labels you'd need a separate channel to for example "record.ZNAM" of a bi/bo record or a "record.ZRST" of an mbbi/o, which gives a string value and when you write that value, you change the first enumeration label. But you can't directly write the labels associated with a value update, just like you can't write the time stamp or alarm info.