-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue in app-pvtable for PV that contains dot in the name the last part of the name is removed #3228
base: master
Are you sure you want to change the base?
Conversation
git-commit-id-maven-plugin:9.0.1 compatibility see https://jitpack.io/com/github/ControlSystemStudio/phoebus/-v4.7.3-g9c0f55d-959/build.log
…-plugin:9.0.1 compatibility see https://jitpack.io/com/github/ControlSystemStudio/phoebus/-v4.7.3-g9c0f55d-959/build.log" This reverts commit 02250d9.
final int sep = name.lastIndexOf('.'); | ||
String fieldVal = name.substring(sep + 1); | ||
//System.out.println("fieldVal=" + fieldVal); | ||
//Test if it in uppercase and max length 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EPICS field names are neither limited to 4 characters nor need they be uppercase.
In very early EPICS releases, field names were internally treated as 4-byte numbers, using a number comparison instead of a proper string comparison to locate fields because the number comp can be performed in a single machine code instruction, whereas string comparisons check character-by-character until you reach the end of the string. For a long time now, EPICS record field names are treated as strings without a 4 character limitation. Check the EPICS base include files, they use char *
for field names, not char[4]
or the like. See also motor record with its IGSET
field, https://epics.anl.gov/bcda/synApps/motor/motorRecord.html
If you create your own record, you can add fields with lower-case field names, there's no rule against that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK , thanks for your answer, but what can we do with our custom datasource ?
Actually, my pv named muscade://PLC_1.AlwaysFALSE is searching for a PLC_1.DESC for the description .. so it's not works.
Can I add a test of datasource as it done for excluding "sim:" and "loc:" datasource from this automatically replacement ?
It's sound good for you if I add a test as :
if (name.startsWith("ca://") {
// automatically replace the last field by DESC
}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the PV name checking is complicated. Only records on an IOC have 'DESC' fields. For Channel Access, that requires a designated channel. For PV Access, the description is already included in the normative type.
But channel access can also be used to access servers based on the PCAS library (or pcaspy), and then there's no "xx.DESC" channel because there is no record. You simply can't tell by looking at the PV name.
That's why we have the show_description
preference setting to disable this madness, since there is no sane solution.
Still, you could indeed look for name.startsWith("ca://"
or also NO protocol prefix when "ca://" is the default. So you would only try to create a separate channel for the "...DESC" when using Channel Access, which might still fail if it's PCAS and not an IOC.
For PV Access, you could get the description from the normative type.
For other protocols ("loc:", "sim:", "muscade:", ...), you can certainly skip the description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it is not really clean to add a test on our custom datasource as it done for "sim:" and "loc:" actually.
What do you prefer between
if (name.startsWith("muscade://") {
// do my own treatment
}
or
if (name.startsWith("ca://") {
// keep the old treatment
}
or improve PV class in adding a method getDescription()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ca://" could be the default. So you need to do something like this:
// Maybe do a better check with regular expression, looking for `[a-z]+://` ?
boolean name_has_no_prefix = name.indexOf("://) < 0;
if (name.startsWith("ca://") || (name_has_no_prefix && PVPool.default_type.equals("ca")) )
{
// For Channel Access, try to create separate ".DESC" channel
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let I try to submit a new proposition for this fix.
Fix issue in app-pvtable for PV that contains dot in the name (should happen with a custom datasource)
Replace last field only if it is a known EPICS fields .
All EPICS fields are in UpperCase andhas a maximum length of 4 characters ex : VAL EGU SCAN ...
Else .DESC is added to the name for example : MyPVName.Information will be MyPVName.Information.DESC for description
and MyPVName.Information.EGU will be MyPVName.Information.DESC for description too .
Improvement on PVTable , the description cell will display the DESC pv name in tooltip behind the description value.