Discussion - EPICS description field in reccaster/recceiver

[tynanford]

Often time the applications that use channel finder for us also would benefit from accessing the EPICS description field. That information could get passed automatically in reccaster

[gabrielpreviato]

I did this in a fork of an older version of the project. You can see the PR here: https://github.com/lnls-sol/recsync/pull/1

I don't know though if this is a desired feature for everyone. The implementation above stores the DESC of all PV's, maybe a smarter way can be implemented.

[tynanford]

Thanks @gabrielpreviato that is really helpful.

It certainly would be a desired feature for us. It appears you are using sqlite instead of channel finder but for folks using CF I know there could be a toggle added like this to enable/disable the feature. https://github.com/ChannelFinder/recsync/blob/master/server/demo.conf#L72

[shroffk]

With the toggle option in place I don't see any harm in including this onto the master branch

[gabrielpreviato]

@tynanford @shroffk I opened a PR for this issue. I'll try to write some code this week for this. But I'll gladly accept some help with testing, since I won't have a full EPICS setup for testing myself in the following weeks.

[tynanford]

Thanks @gabrielpreviato ! I can help with testing. I tested your current changes and created a pull request into your adds-desc-field branch for some channel finder push additions. With those changes I am seeing the recordDesc in CF

@shroffk can comment better on the network traffic with the DESC field. But I think the EPICS DESC field is limited to 40 chars? So maybe that is ok?

[tynanford]

A few observations

• updating the description field of an existing PV updates it in CF too, so that's good
• deleting the description field of an existing PV doesn't remove the recordDesc field in CF
• A PV DESC field bigger than 40 chars does indeed get passed on to recceiver in full. So disregard my previous comment on that

[gabrielpreviato]

@tynanford I think the second point you mentioned can be caused by the checking if the DESC is empty, since deleting the field will make the RecCaster send an empty string. I never worked with the CF module itself, I only worked with the SQLite part. Can you explain shortly why "CF doesn't like empty or null properties"?

Anyway, I added an else statement, so if the recordDesc modified value is empty, it won't create a recordDesc field in the end.

[tynanford]

Channel finder considers it a bad HTTP request if a property is null or empty. Which means the entire push to channel finder won't work and no channels get added. Probably something that could be handled more gracefully on the CF end

2022-03-20 07:22:12.026 ERROR 3051 --- [http-nio-8080-exec-4] gov.bnl.channelfinder.ChannelManager     : The property with the name description has value  is null or empty

org.springframework.web.server.ResponseStatusException: 400 BAD_REQUEST

Also,

• I once again take back what I said about the DESC field character length :) ... I was mistakenly editing the INFO field instead of the DESC field yesterday. Using EPICS 7.0.6 I can confirm it won't allow DESC > 40 characters and the long DESC doesn't make it to recceiver
• There is a typo in your latest commit here - https://github.com/gabrielpreviato/recsync/blob/adds-desc-field/server/recceiver/cfstore.py#L354 & and line 377 "propery" should be "properties"

After changing the typo I still don't see the recordDesc get deleted, not sure why. I can look more later today.

[tynanford]

@gabrielpreviato I created a new pull request in your repo https://github.com/gabrielpreviato/recsync/pull/2

Things are looking good from my end with channel finder. I haven't used sqlite for recceiver. Is that tested or should I test that too?

[gabrielpreviato]

@tynanford Good to hear that. I also tested the CF here and everything looks good. I added some code for the sqlite module, I'll test it on my side, but if you can also try it, it would be great.

[tynanford]

@gabrielpreviato I tested the sqlite plugin and see the rdesc being filled in, everything looks good to me

[tynanford]

@gabrielpreviato I realized I was using your lnls reccaster code instead of your personal repo.

Upon compiling the pull request repo there were issues. 2 are fixed in this: https://github.com/gabrielpreviato/recsync/pull/3

But this one still needs fixing:

../dbcb.c: In function ‘pushRecord’:
../dbcb.c:97:105: error: request for member ‘desc’ in something not a structure or union
   97 |                 ret = casterSendAlias(caster, rid, subent.precnode->recordname, subent.precnode->precord.desc);
      |                                                                                                         ^

[gabrielpreviato]

@tynanford There was a pointer mistake, now it's building correctly.

[tynanford]

@gabrielpreviato Thanks! Sorry I haven't had time to look into this until now.

Things look good with the new changes.

Doing a test with:

• new recceiver
• 1 IOC with new reccaster
• 1 IOC with current reccaster

The IOC with the older reccaster doesn't work correctly. I'll take a look more if there are ways to make the new version of recceiver backwards compatible with old clients.

In the meantime, @shroffk @mdavidsaver do you have any thoughts on the pull request? https://github.com/ChannelFinder/recsync/pull/53

And is maintaining backwards compatibility necessary?

[mdavidsaver]

And is maintaining backwards compatibility necessary?

Let me spin that question around. What happens if compatibility isn't maintained? From the prospective of say, a site with >100 installed IOCs. How is this communicated? Is there a migration plan?

I think this is an important thought exercise, quite apart from whether a compatibility break is called for in this instance.

wrt. forward compatibility. There are a couple of ways that new information can be accommodated compatibly by the recsync protocol.

• Pretend it is an info tag. Existing recceiver processes as normal.
• Add a new msgid. eg. Field Update. Existing recceiver ignores this unknown msgid. (cf. CastReceiver.ignoreBody)
• Append to an existing message body. eg. Add rdlen after rname. Existing recceiver only processes the prefix it knows about, and ignores the rest.
• Add a new atype in Add Record. (other than 0 or 1) Existing recceiver will ignore

I think that any of these could be used in this situation.

Which of these is chosen may depend on the answer to a question. What to do if DESC changes?

[tynanford]

Thanks for looking into this and for the comments @mdavidsaver It sounds like there are several ways to maintain compatibility so that's great. I don't have a lot of time this week to work on it but will try to take a look at what would work best. Let me know @gabrielpreviato if you plan to look into it too

[shroffk]

Hello,

Backward compatibility, especially with old reccasters is very important... since it is non trivial to upgrade and rebuild hundreds or IOC's. With recceiver I think we have a bit more flexibility.

We might want to make a release before making these changes.

@tynanford I have added recceiver as a project for the upcoming code-a-thon https://github.com/ControlSystemStudio/phoebus/wiki/Epics-tools-and-services-code-a-thon-project-ideas

[tynanford]

Thanks @shroffk . I will be able to attend the code-a-thon for at least 2 days and can help with recsync/channel finder/phoebus stuff

[tynanford]

@mdavidsaver @shroffk See here: https://github.com/ChannelFinder/recsync/pull/54

I decided to try the strategy to pretend the recordDesc is an INFO tag. It works well. Tested and confirmed these set ups all work as expected. What are your thoughts?

• Channel Finder
  • New recceiver & IOC w/ new reccaster
  • recordDesc toggle adds the description field
  • Adding recordDesc to infotags list also adds the description field
  • Old recceiver & IOC w/ new reccaster
  • old recceiver works normally, doesn't care. Still can get recordDesc in CF using infotags list
  • New recceiver & IOC w/ old reccaster
  • old reccaster IOC works normally and PVs make it to CF, just doesn't send recordDesc
  • Changing DESC updates recordDesc
  • Deleting DESC doesn't delete recordDesc. This is common behavior with any INFO tags though
• sqlite
  • recordDesc gets added to recinfo table

@gabrielpreviato The disadvantage I see is for sqlite. recordDesc is in the recinfo table instead of record table. What do you think?

[tynanford]

@shroffk @mdavidsaver

Do you have thoughts on this? We have this pull request running in 3 production IOCs right now and it is working well https://github.com/ChannelFinder/recsync/pull/54

If you prefer I could change this line (https://github.com/ChannelFinder/recsync/pull/54/commits/f575b1992d2dd3ddce795ada43166329a7321365#diff-a3ca2db476ed49c9f985a2122e1a7454ae3f89b02581391a91e5c362bb881b84R115) to be cleaner, something like this:

    /* send desc as INFO tag */
    if(prec->desc && prec->desc[0]!='\0')
        ret = casterSendInfo(caster, rid, "recordDesc", prec->desc);