search

epics-modules / mrfioc2

EPICS driver for Micro Research Finland event timing system devices
http://epics-modules.github.io/mrfioc2/
Other
8 stars 30 forks source link

Generated record names use illegal characters #8

Closed anjohnson closed 7 years ago

anjohnson commented 8 years ago

The list of characters that are (meant to be) legal in record names is given in section 6.3.2 of the EPICS Application Developers' Guide. That list does not include the brace characters { or } that are used by some of the substitution files delivered with this module. Please consider changing the public version of this module to remove the brace characters; either angle brackets < and > or square brackets [ and ] might be suitable alternatives.

In the future I would like to be able to use braces at the beginning and end of PV link strings to distinguish simple PV (record) names from structured data encoded in the PV link string. My current plans shouldn't affect the record names generated by this module (since the braces appear in the middle of the names), but the bad example set here might mislead a site into using braces in a manner that will not work after my changes have been implemented.

mdavidsaver commented 8 years ago

I think the ship has sailed on {} as allowed characters in a record name back in 2012 (blame Bob). The only place it causes any problems is with snc/seq, and is easily worked around since the sequencer doesn't do recursive substitution.

So rather than introduce unnecessary complication for NSLS2 I think the proper fix is to update the appdev guide.

In the future I would like to be able to use braces at the beginning and end of PV link strings to distinguish simple PV

This is a nice idea, but realistically this will have to be done in the same way as the server-side filters by requiring "record.FLD{}". If you want to be clever, nsls2 record names should be of the form "AAA{BBB}CCC" so they never begin with '{' or end with '}'.

anjohnson commented 8 years ago

I reject your "realistically" and reiterate my own idea. I might be willing to allow braces in record names as long as they aren't the first or last characters in the name, which allows the NSLS2 spec. However just because a future version of the spec might allow them doesn't mean your driver has to encourage them — I have been told that PSI changes your substitution files to remove them, and I will be working on Ned to have him do that here too, so whatever you deliver someone will always have to change your driver before using it.

The new (optional) PV Link field syntax can be encoded using JSON, which the parser could easily distinguish providing that all record names follow the above rule. Basically if the first non-space character in the link value is { and the last non-space is } then the whole string would be parsed as JSON since such a string cannot be a legal PV name, with or without field modifiers. With no braces the IOC will use the old parser.

I'm also considering if we can introduce an array initializer PV Link syntax for constant links where the first non-space is [, the last non-space is ] and there is at least one , in between. Brackets are legal characters in record names, but a comma is not. A PV name with or without an array filter can start and end with brackets, but there's no legal way I can see to get a comma in between them. People have been asking for constant link initalization of array and string values for many years, so I think this is worth doing if it's possible.

mdavidsaver commented 8 years ago

By design all of the {} are confined to .substitutions files. So all we're talking about here is copying evrMrmApp/Db/evr-vmerf-230.substitutions as evrMrmApp/Db/evr-vmerf-230-aps.substitutions, which I would be happy to include as per S5.5.

anjohnson commented 8 years ago

Comprendo, and I appreciate that aspect of the design. I was just asking you to consider changing the site-neutral version of the file in future releases so that new users won't see the example names from NSLS-2 and (without understanding the naming rule discussed above) start including braces in their own PV naming conventions. Providing an NSLS-2-specific substitutions file that does use braces should be less likely to lead to that happening.

I know this is an incredibly nit-picky and small change to ask for, I'm just trying to avoid leading newcomers astray. It's your code though, so I'll shut up about this now.

mdavidsaver commented 7 years ago

I'm closing this issue as "won't fix". I appreciate the argument, but don't find it compelling enough to change existing names. The NSLS2 convention is here to stay, warts and all.