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

lutX should not be used #172

Closed Insomnia1437 closed 3 days ago

Insomnia1437 commented 1 month ago

When mapping EVR output to Prescaler 1, OPI reported READ_ALARM. real output is not affected

It looks like the info field of lutX should be reserved as an "unknown" option.

I am looking into it and will fix it when I have time

https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/mrfCommon/src/devlutstring.cpp#L90-L96

https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/evrMrmApp/Db/mrmevrout.db#L439-L446

See

image

jerzyjamroz commented 1 week ago

@Insomnia1437 , as far I can notice quickly, mrfioc2/mrfCommon/src/devlutstring.cpp Lines 90 to 96 in 5b0c1a1 can be deleted. It looks like an artifact, what is the purpose of that?

mdavidsaver commented 1 week ago

info(lutX, ...) was intended to allow the default "unknown" string to be customized. Of course such customization is not a requirement.

https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/mrfCommon/src/devlutstring.cpp#L80

This being the default string when no entry matches

https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/mrfCommon/src/devlutstring.cpp#L141 https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/mrfCommon/src/devlutstring.cpp#L150-L154 https://github.com/epics-modules/mrfioc2/blob/5b0c1a1739a210ffc8bfe1b1118723cdd9fd6ee2/mrfCommon/src/devlutstring.cpp#L162-L163

Insomnia1437 commented 6 days ago

As Michael explained, I think it's more appropriate to keep this customization. I suggest changing the hard-coded lutX to lutXX to increase future scalability.

jerzyjamroz commented 6 days ago

Ok, so I propose PR https://github.com/epics-modules/mrfioc2/pull/175 with lutX changed to lut_ to avoid the letters.