Skip to content
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

MENU and DEVICE fields treated ENUMs in 3.15 #17

Open
dhickin opened this issue Sep 12, 2015 · 7 comments
Open

MENU and DEVICE fields treated ENUMs in 3.15 #17

dhickin opened this issue Sep 12, 2015 · 7 comments

Comments

@dhickin
Copy link
Contributor

dhickin commented Sep 12, 2015

In 3.15 code dbChannelFinalFieldType(dbChannel) is used in place of dbAddr.field_type. This returns the final_type field which is a DBR type. The conversion converts DBF_MENU and DBF_DEVICE to enums and these are treated incorrectly.

@ralphlange
Copy link
Contributor

I don't think so.
dbChannel.final_type is the final field type, so it is a DBF type. (Note that all comparisons in 3.15/dbUtil.cpp compare dbChannelFinalFieldType(dbChannel) against DBF constants.)
The matching DBR type would be returned by the macro dbChannelFinalCAType().
Where does the conversion go wrong? Menus (and device support options as a special case) should be represented as enums, shouldn't they?

@dhickin
Copy link
Contributor Author

dhickin commented Sep 14, 2015

I'm looking at (and building against) 3.15.2.

In dbChannel.c open() sets chan->final_type to probe.field_type which is dbChannelExportType(chan). This in turn is defined as

#define dbChannelExportType(pChan) ((pChan)->addr.dbr_field_type)

All this assuming no filters. So if addr.field_type==DBF_ENUM (9),DBF_MENU (10) or DBF_DEVICE (11) chan->final_type will be =DBR_ENUM (9). I have checked both the values and the path through open with debug.

Although the value of dbChannelFinalFieldType(dbChan) is compared against DBF types, it is itself therefore a DBR type. This means every time we have a DBF_MENU or DBF_DEVICE comparison we go instead through the DBF_ENUM branch instead. Examples include putting the correct strings in the choices field in createPVStructure(), getting the right index value in get() and putting the index to the right place in put. Just search for DBF_MENU and DBF_DEVICE in dbUtil.cpp.

So although a get, for example, of a channel to a device and menu field will correctly return an enum_t, the choices and possibly index will be wrong.

I have a fix which I was about to send in a pull request with a number of other issues.

@ralphlange
Copy link
Contributor

If that is true, it is a bug in EPICS Base.
dbChannelFinalFieldType(dbChan) is supposed to get the DBF type.
Please do not fix this in pvaSrv.

@dhickin
Copy link
Contributor Author

dhickin commented Sep 14, 2015

Surely we have too. If EPICS base has a bug we have to work round it don't we?

My fix in my commits is to replace calls of dbChannelFinalFieldType with a function which at least in the interim can wrap the fix to dbChannelFinalFieldTyp. I see no issue in having a function in pvaSrv which gets the final field from a dbChannel, even if the one in base is supposed to do this. In any case I'd rather have the code working correctly.

@ralphlange
Copy link
Contributor

The bug in Base was introduced by revision 12658 (http://bazaar.launchpad.net/~epics-core/epics-base/3.15/revision/12658#src/ioc/db/dbChannel.c) which obviously fixed a behaviour for LINK fields while creating trouble for other special fields.
I created a bug on LP for this. (https://bugs.launchpad.net/epics-base/+bug/1495417)

@ralphlange
Copy link
Contributor

Your workaround sounds appropriate. That function's implementation can be ifdef'd to do the right thing once the issue is fixed in Base.

dhickin added a commit to dhickin/pvaSrv that referenced this issue Sep 14, 2015
When creating a pvAccess channel to a field of a record with a VAL field
of type DBF_ENUM whose DBF type is DBF_DEVICE or DBF_MENU, for example,
the DTYP or SCAN fields of a bi/bo/mbbi/mbbo record, and performing an
operation involving its "values.choices" field (Channel Get, Put or
Monitor) the "choices" field is filled in incorrectly with the values from the
record's "VAL" field. Fix this for EPICS base R3.14. Add equivalent
change for R3.15 which will also require fix for epics-rip#17.

Fixes epics-rip#8
dhickin added a commit to dhickin/pvaSrv that referenced this issue Sep 14, 2015
In the 3.15 code (dbChannel) is used where
dbAddr.field_type is used in 3.14. dbChannelFinalFieldType returns the
final_type field of dbChannel, which is a DBR type, and so for a
DBF_MENU or DBF_DEVICE field returns DBR_ENUM. These field types are
treated wrongly as a result (epics-rip#17). Replace dbChannelFinalFieldType with
a new function. Implement this by returning DBF field when no filters in
place, dbChannelFinalFieldType otherwise.
@dhickin
Copy link
Contributor Author

dhickin commented Sep 14, 2015

I have implemented the above function as:

static short dbChannelFinalDBFType(dbChannel *dbChan)
{
    if (ellCount(&dbChan->filters) == 0)
        return dbChannelFieldType(dbChan);
    else
        return dbChannelFinalFieldType(dbChan);
}

thus it will be correct when there are no filters and the same as the previous implementation when there are. This should work for other DBF type even when there is filtering. I could look at filtered case, but it may be better for V3 experts to do this (or generally change the implementation as they see fit). I've not used the commit message to close it for this reason.

I've tested this fix across a range of types without filtering present and it works for me on both R3.14.12.3 and R3.15.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants