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

Add new VType class in Phoebus to manage Description in Enum and manage Labels properly in PVTable #3237

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

katysaintin
Copy link
Contributor

@katysaintin katysaintin commented Jan 15, 2025

Hello @shroffk and @kasemir ,

Here is my proposal for fix Description and Enum values in waiting epics-base/epicsCoreJava#141
resolution. Perhaps it will help you to fix it in epics core-vtype module .

  • Add a new interfaces and classes in phoebus core-vtype :
  1. DescriptionProvider to manage description on VBoolean and VEnum (normally VType should implement it) .
  2. EnumProvider to manage ONAM and ZNAM for Boolean value and get EnumDisplay for example "ON, OFF" ..
  3. AdvancedBoolean that implements DescriptionProvider & EnumProvider and extends VBoolean
  4. AdvancedEnum that implements DescriptionProvider and extends VEnum

In PVTable I update all the code in order to get description by this way instead of PV instanceof DisplayProvider
I also use EnumDisplay to display Boolean value, because actually it display VBoolean.toString

Here is a screenshot on PVTable with Muscade OPCUA DataSource
MuscadeDataSourceOPCUA

@kasemir
Copy link
Collaborator

kasemir commented Jan 15, 2025

There are several ways to do this, so we can argue about this for a long time, but here's what I would do, and this has to be done in the VTypes:

DescriptionProvider is perfect as you have it.

Remove getDescription from the existing Display and instead have it implement DescriptionProvider, so it basically looks the same as before but with an inherited getDescription.
Have EnumDisplay also implement DescriptionProvider, so now the VEnum.getDisplay().getDescription() gets you the new info.

That would avoid adding new a AdvancedEnum, AdvancedBoolean and EnumProvider.

Note that IOCs use VEnum for boolean data. VBoolean is just true/false. Boolean records (bi, bo) use a VEnum with two options, that's how you can have labels for the two states beyond just true/false.
If your Muscade PVs have boolean data with added information like labels and now a description, use VEnum just like EPICS does.

Doing some intermediate work here until VTypes are updated means it has to be re-done in 2 weeks.

@shroffk
Copy link
Member

shroffk commented Jan 15, 2025

@katysaintin with the previous PR #3228 merged I hope you are no longer stuck

I think I would like a bit more time on this issue primarily to understand the impact on the existing code.

@katysaintin
Copy link
Contributor Author

@kasemir Thank you for your code review .
I can move my 2 classes AdvancedBoolean and AdvancedEnum in my Muscade project , but I thought it was useful to provide it for other datasource as an exemple.

For Boolean , I use Enum value instead, but some widget and rule use ONAM & ZNAM such as LED Boolean Button or boolean rule . If I use an enum it's become pv == 0 instead of !pv0 , that why I want to use VBoolean.

For ,boolean EPICS , I'm not sure, are bo and bi record are boolean or enum ? is EnumDisplay get ONAM & ZNAM ?

Anyway, thank you so much both of you @shroffk @kasemir for your help.

Katy

@katysaintin
Copy link
Contributor Author

@katysaintin with the previous PR #3228 merged I hope you are no longer stuck

I think I would like a bit more time on this issue primarily to understand the impact on the existing code.

Yes No problem, I'm not stuck anymore, I can keep as it in my fork for instant.

@kasemir
Copy link
Collaborator

kasemir commented Jan 16, 2025

For ,boolean EPICS , I'm not sure, are bo and bi record are boolean or enum ? is EnumDisplay get ONAM & ZNAM ?

Yes, channels to bo and bi records use VEnum with two labels/choices in the EnumDisplay.

@katysaintin katysaintin changed the title Add new VType class in Phoebus to manage Description and Enum Labels properly in PVTable Add new VType class in Phoebus to manage Description in Enum and manage Labels properly in PVTable Jan 17, 2025
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

Successfully merging this pull request may close these issues.

3 participants