ioc: improve long string detection

[mdavidsaver]

Attempt to improve automatic handling of "long strings". At least to handle the $ case.

I had hoped that I could come up with a way to detect the effects of $ or similar server side filters by inspecting the types stored in the dbChannel and dbAddr structs. However, I've almost reached the point of giving up. Between the mangling done in dbChannelCreate(), and in various rset::cvt_dbaddr(), I don't think this can reliably be done without extending dbChannel.

And there are also cases like waveformRecord with FTVL=CHAR, where .VAL$ has never been supported because the underlying field type isn't DBR_STRING.

@anjohnson @coretl fyi.

[coretl]

@mdavidsaver thanks, this fixes my immediate problem, reading lsi_record.VAL$ as a string. I tried it out using the wheel generated as an artifact from this PR, and pythonSoftIOC from https://github.com/dls-controls/pythonSoftIOC/pull/132

I'll consider what the best mix of https://github.com/dls-controls/pythonSoftIOC/pull/132 and https://github.com/dls-controls/pythonSoftIOC/pull/133 is to merge when there's a new release of pvxslibs to point at

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.898 completed (commit https://github.com/mdavidsaver/pvxs/commit/324c44af22 by @mdavidsaver)

[mdavidsaver]

... when there's a new release of pvxslibs to point at

Unless I hear cries of horror from @anjohnson this week, I'll merge and release.

[anjohnson]

I haven’t checked the code since I’m only using an iPad right now, but I believe it’s legal to follow a $ modifier with a subarray […] modifier which wouldn’t work with this.

Is it not possible to support lsi.VAL as a long string directly by looking at the field size? I thought there was a way to do that since copying long strings through a DB link is possible IIRC, maybe look at the lsi softDev support for ideas? Sorry I don’t have time to look myself right now.

[mdavidsaver]

Is it not possible to support lsi.VAL as a long string directly by looking at the field size? ...

Probably, although this does raise the issue of retraining users not to append $, and live with a difference between caget and pvget. Still, perhaps this is the way forward.

I have also found myself considering always mapping DBF_CHAR array to PVA string. DBF_UCHAR would continue to map to uint8_t[] for actual byte arrays. Of course, this would also be a difference with both CA and earlier QSRV...

[coretl]

Is it not possible to support lsi.VAL as a long string directly by looking at the field size? ...

Probably, although this does raise the issue of retraining users not to append $, and live with a difference between caget and pvget. Still, perhaps this is the way forward.

Personally I would prefer this. For my 3 use cases it would be great if pvget worked without the $:

• pvget lsi.VAL
• pvget calc.INPA
• pvget anyrecord.NAME

I would be happy to move any of my usage of long strings in a waveform record to use lsi/lso records if this was supported.

I have also found myself considering always mapping DBF_CHAR array to PVA string. DBF_UCHAR would continue to map to uint8_t[] for actual byte arrays. Of course, this would also be a difference with both CA and earlier QSRV...

For waveform records? I guess some fast ADCs might produce signed 8-bit data via waveform records, although that's pretty rare.

[coretl]

I have also found myself considering always mapping DBF_CHAR array to PVA string. DBF_UCHAR would continue to map to uint8_t[] for actual byte arrays. Of course, this would also be a difference with both CA and earlier QSRV...

How about an opt-in for this, like Q:form = String on the waveform record to turn on mapping of DBF_CHAR array to PVA string?

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.900 completed (commit https://github.com/mdavidsaver/pvxs/commit/f034043f70 by @mdavidsaver)

[mdavidsaver]

Is it not possible to support lsi.VAL as a long string directly by looking at the field size? ...

Probably, although this does raise the issue of retraining users not to append $, and live with a difference between caget and pvget. Still, perhaps this is the way forward.

Personally I would prefer this. For my 3 use cases it would be great if pvget worked without the $:

I've rewritten around this idea. I think the changes in the test code illustrate things.

Reading test:this:is:a:really:really:long:record:name.NAME would no longer truncate. Similarly for long link strings. The current behavior with $ is preserved.

With this change, access to lsi should "just work" without a $. Behavior with a $ is unchanged (treated as int8_t[]).

For waveform records? I guess some fast ADCs might produce signed 8-bit data via waveform records, although that's pretty rare.

This is a point...

How about an opt-in for this, like Q:form = String on the waveform record ...

I think I have figured out a way to make this work without adding another dbFindRecord() with 3.15 (3.16.1 adds dbInitEntryFromRecord() which O(0)).

record(waveform, "test:long:str:wf") { field(FTVL, "CHAR") field(NELM, "128") info(Q:form, "String") }

[mdavidsaver]

@anjohnson I've opted for "mangling" the dbChannel in between dbChannelCreate() and dbChannelOpen(). The logic is effectively a copy of what is currently done in dbChannelCreate(). I don't especially like doing this, and worry about compatibility going forward if/when dbChannelCreate() changes. However, the only alternative I see would be parsing and mangling the PV name string prior to dbChannelCreate(), which I like less.

Also, at the moment I'm the "mangling" logic doesn't test ellCount(&chan->filters) since dbChannelCreate() doesn't. However, I'm considering restricting "mangling" to only when no filters are used. Mainly because I don't have a good idea of what to test, and how things might go wrong.

[anjohnson]

I haven't tried to compare the code, but I have an impression that this is cleaner than the previous version (although I don't remember looking at that too closely). I was hoping we could offer a solution such that users at PVA-only sites won't need to worry about $ or the 40-char string size limit anywhere near as much, and this does seem to offer that.

Could adding an alternate dbChannelCreateLS() routine to Base remove the concern about later modifications to the dbChannel code? It would do something like always exposing DBF_STRING fields to the caller as an array of DBR_CHAR, which I think is what you're effectively doing. There may be a problem with that, but I can't see it right now.

My one concern is that I assume adding info(Q:form, "String") applies to all fields of the record. Can it be configured any more finely than that? An aSub record might have one field that holds a long string and another with an array of int8 values. This seems likely to be rare, but not impossible (so there would probably be a tech-talk question about supporting it 2 weeks after you release this change!).

[mdavidsaver]

Could adding an alternate dbChannelCreateLS() routine to ... do something like always exposing DBF_STRING

How about adding a flags argument? With one flag requesting automatic promotion of DBF_STRING to DBF_CHAR[].

It will also be necessary to give some indication that this promotion has happened. This is really the missing piece in dbChannelCreate() now which would remove most of the ambiguity of intent wrt. long string vs. array of signed bytes.

I was thinking to add a flag like dbChannel::longString, although with a new function there are other possibilities to get this information back to the caller.

My one concern is that I assume adding info(Q:form, "String") applies to all fields of the record.

Yup. I didn't think of this back when Q:form was added. This tag was used by pva2pva as well. I could limit this hint to VAL.

Of course aSub is an extra special case, which I'm inclined to ignore for the present.

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.902 completed (commit https://github.com/mdavidsaver/pvxs/commit/efaff700b4 by @mdavidsaver)

[mdavidsaver]

My one concern is that I assume adding info(Q:form, "String") applies to all fields of the record.

Yup. I didn't think of this back when Q:form was added. This tag was used by pva2pva as well. I could limit this hint to VAL.

I have updated things such that the Q:form hint only applies when dbIsValueField().

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.906 completed (commit https://github.com/mdavidsaver/pvxs/commit/cd06817c7e by @mdavidsaver)

[mdavidsaver]

@coretl If you have time, can you take another look at this? Despite 1.2.1 only being 6 days old, after working through clang-tidy results, I am thinking about 1.2.2. If it is ready, I'd like to include this PR as well.

[AppVeyorBot]

:x: Build pvxs 1.0.909 failed (commit https://github.com/mdavidsaver/pvxs/commit/daf36adbac by @mdavidsaver)

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.909 completed (commit https://github.com/mdavidsaver/pvxs/commit/daf36adbac by @mdavidsaver)

[coretl]

I've installed the python3.10 linux wheel from https://github.com/mdavidsaver/pvxs/actions/runs/5304865866 and it seems to work fine for me, no $ characters needed for my use cases. Thanks, looking forward to 1.2.2!