kasemir / org.csstudio.display.builder

Update of org.csstudio.opibuilder.*
Eclipse Public License 1.0
2 stars 10 forks source link

Serious problems with tooltips and rules (in TextUpdate) #364

Closed claudio-rosati closed 6 years ago

claudio-rosati commented 6 years ago

Hello Kay,

a colleague has found a problem with tooltip and rules, and I then I've found more trying to reproduce it. Please refers to the attached OPIs:

CSSTUDIO-630.zip

CSSTUDIO-630-1.bob has:

If you open with the editor and then run it everything works fine.

In CSSTUDIO-630-1.bob I have set the Tooltip to "-" and created a rule identical to the one updating the Label's Text. Here already I have a problem, because when I set a rule's value I got the following message:

2018-03-13 12:21:03.386 WARNING [Thread 1] org.csstudio.display.builder.model.WidgetProperty (getPath) - Cannot determine path to tooltip for Widget 'Text Update' (textupdate)

Now If you open the CSSTUDIO-630-2.bob file in the editor you should have the following error:

2018-03-13 12:23:11.565 WARNING [Thread 67] org.csstudio.display.builder.model.MacroizedWidgetProperty (getValue) - Widget 'Text Update' (textupdate) property tooltip cannot expand macros for '$(pv_name)
$(pv_value)'
java.lang.NullPointerException
    at org.csstudio.display.builder.model.macros.MacroOrPropertyProvider.getValue(MacroOrPropertyProvider.java:72)
    at org.csstudio.display.builder.model.macros.MacroHandler.replace(MacroHandler.java:109)
    at org.csstudio.display.builder.model.macros.MacroHandler.replace(MacroHandler.java:59)
    at org.csstudio.display.builder.model.MacroizedWidgetProperty.getValue(MacroizedWidgetProperty.java:148)
    at org.csstudio.display.builder.model.WidgetProperty.clone(WidgetProperty.java:91)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.propIDToNewProp(RulesWidgetProperty.java:63)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.readExpressions(RulesWidgetProperty.java:281)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.readFromXML(RulesWidgetProperty.java:226)
    at org.csstudio.display.builder.model.WidgetConfigurator.configureAllPropertiesFromMatchingXML(WidgetConfigurator.java:111)
    at org.csstudio.display.builder.model.WidgetConfigurator.configureFromXML(WidgetConfigurator.java:88)
    at org.csstudio.display.builder.model.widgets.TextUpdateWidget$CustomWidgetConfigurator.configureFromXML(TextUpdateWidget.java:83)
    at org.csstudio.display.builder.model.persist.ModelReader.createWidget(ModelReader.java:239)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidget(ModelReader.java:212)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidgetsAllowingRetry(ModelReader.java:169)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidgets(ModelReader.java:141)
    at org.csstudio.display.builder.model.persist.ModelReader.readModel(ModelReader.java:122)
    at org.csstudio.display.builder.model.persist.ModelLoader.loadModel(ModelLoader.java:63)
    at org.csstudio.display.builder.model.persist.ModelLoader.loadModel(ModelLoader.java:50)
    at org.csstudio.display.builder.editor.rcp.DisplayEditorPart.doLoadModel(DisplayEditorPart.java:254)
    at org.csstudio.display.builder.editor.rcp.DisplayEditorPart.lambda$4(DisplayEditorPart.java:245)
    at java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1590)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

2018-03-13 12:23:11.566 INFO [Thread 67] org.csstudio.display.builder.model.MacroizedWidgetProperty (getValue) - Widget 'Text Update' (textupdate) 'tooltip' is not fully resolved: $(pv_name)
$(pv_value)

The funny thing is that inside the file "$(pv_name)$(pv_value)" doesn't exist.

If you open the file in runtime, you should have the same error, but everything works as expected.

In the last OPI, CSSTUDIO-630-3.bob, the Text Update is left with its default Tooltip value, but with the usual rule to change it depending on the localPV value.

In that case, when you open the file in the editor you'll have the same error as before. If you open it in runtime, the following appears instead:

2018-03-13 12:29:35.338 WARNING [Thread 79] org.csstudio.display.builder.model.MacroizedWidgetProperty (getValue) - Widget 'Text Update_1' (textupdate) property tooltip cannot expand macros for '$(pv_name)
$(pv_value)'
java.lang.NullPointerException
    at org.csstudio.display.builder.model.macros.MacroOrPropertyProvider.getValue(MacroOrPropertyProvider.java:72)
    at org.csstudio.display.builder.model.macros.MacroHandler.replace(MacroHandler.java:109)
    at org.csstudio.display.builder.model.macros.MacroHandler.replace(MacroHandler.java:59)
    at org.csstudio.display.builder.model.MacroizedWidgetProperty.getValue(MacroizedWidgetProperty.java:148)
    at org.csstudio.display.builder.model.WidgetProperty.clone(WidgetProperty.java:91)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.propIDToNewProp(RulesWidgetProperty.java:63)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.readExpressions(RulesWidgetProperty.java:281)
    at org.csstudio.display.builder.model.properties.RulesWidgetProperty.readFromXML(RulesWidgetProperty.java:226)
    at org.csstudio.display.builder.model.WidgetConfigurator.configureAllPropertiesFromMatchingXML(WidgetConfigurator.java:111)
    at org.csstudio.display.builder.model.WidgetConfigurator.configureFromXML(WidgetConfigurator.java:88)
    at org.csstudio.display.builder.model.widgets.TextUpdateWidget$CustomWidgetConfigurator.configureFromXML(TextUpdateWidget.java:83)
    at org.csstudio.display.builder.model.persist.ModelReader.createWidget(ModelReader.java:239)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidget(ModelReader.java:212)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidgetsAllowingRetry(ModelReader.java:169)
    at org.csstudio.display.builder.model.persist.ModelReader.readWidgets(ModelReader.java:141)
    at org.csstudio.display.builder.model.persist.ModelReader.readModel(ModelReader.java:122)
    at org.csstudio.display.builder.model.persist.ModelLoader.loadModel(ModelLoader.java:63)
    at org.csstudio.display.builder.model.persist.ModelLoader.loadModel(ModelLoader.java:50)
    at org.csstudio.display.builder.rcp.RuntimeViewPart.loadModel(RuntimeViewPart.java:567)
    at org.csstudio.display.builder.rcp.RuntimeViewPart.lambda$3(RuntimeViewPart.java:508)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

2018-03-13 12:29:35.339 INFO [Thread 79] org.csstudio.display.builder.model.MacroizedWidgetProperty (getValue) - Widget 'Text Update_1' (textupdate) 'tooltip' is not fully resolved: $(pv_name)
$(pv_value)
2018-03-13 12:29:35.454 WARNING [Thread 81] org.csstudio.display.builder.runtime.WidgetRuntime (startScripts) - Rule failed to compile
Display 'CSSTUDIO-630-3', Widget 'Text Update_1' (textupdate), New Rule
java.lang.Exception: Cannot compile rule: textupdate:Text Update_1:New Rule.rule.py
   1: ## Script for Rule: New Rule
   2: 
   3: from org.csstudio.display.builder.runtime.script import PVUtil
   4: 
   5: ## Process variable extraction
   6: ## Use any of the following valid variable names in an expression:
   7: ##     pv0
   8: ##     pvInt0
   9: ##     pvStr0
  10: ##     pvSev0
  11: ##     pvLegacySev0  [DEPRECATED]
  12: 
  13: pvInt0 = PVUtil.getLong(pvs[0])
  14: 
  15: ## Script Body
  16: if pvInt0 == 0:
  17:     widget.setPropertyValue('tooltip', "NO TOOLTIP")
  18: elif pvInt0 > 0:
  19:     widget.setPropertyValue('tooltip', "TOOLTIP")
  20: else:
  21:     widget.setPropertyValue('tooltip', "$(pv_name)
  22: $(pv_value)")

    at org.csstudio.display.builder.runtime.script.internal.RuntimeScriptHandler.compileScript(RuntimeScriptHandler.java:121)
    at org.csstudio.display.builder.runtime.script.internal.RuntimeScriptHandler.<init>(RuntimeScriptHandler.java:140)
    at org.csstudio.display.builder.runtime.WidgetRuntime.startScripts(WidgetRuntime.java:269)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)
Caused by: SyntaxError: ("no viable alternative at character '$'", ('textupdate:Text Update_1:New Rule.rule.py', 22, 0, '$(pv_value)")\n'))

    at org.python.core.ParserFacade.fixParseError(ParserFacade.java:95)
    at org.python.core.ParserFacade.parseExpressionOrModule(ParserFacade.java:136)
    at org.python.util.PythonInterpreter.compile(PythonInterpreter.java:320)
    at org.csstudio.display.builder.runtime.script.internal.JythonScriptSupport.compile(JythonScriptSupport.java:204)
    at org.csstudio.display.builder.runtime.script.internal.ScriptSupport.compile(ScriptSupport.java:79)
    at org.csstudio.display.builder.runtime.script.internal.RuntimeScriptHandler.compileScript(RuntimeScriptHandler.java:117)
    ... 5 more

and the tooltip doesn't behave as expected.

kasemir commented 6 years ago

I'll have to look at it next week. Meanwhile, in the code that generates scripts from rules, you could locate the part that creates the else setPropertyValue(prop, the_default_value) and modify it so that string default values are corrected, each return must be formatted as '\n'.

claudio-rosati commented 6 years ago

That is part of the problem I think, because, as you can see from CSSTUDIO-630-3.bob, there is no such value in the XML:

<?xml version="1.0" encoding="UTF-8"?>
<display version="2.0.0">
  <name>CSSTUDIO-630-3</name>
  <widget type="bool_button" version="2.0.0">
    <name>Boolean Button_1</name>
    <pv_name>loc://A-BOOLEAN(0)</pv_name>
    <x>20</x>
    <y>20</y>
  </widget>
  <widget type="label" version="2.0.0">
    <name>Label_1</name>
    <text>-</text>
    <x>20</x>
    <y>70</y>
    <rules>
      <rule name="New Rule" prop_id="text" out_exp="false">
        <exp bool_exp="pvInt0 == 0">
          <value>NO TOOLTIP</value>
        </exp>
        <exp bool_exp="pvInt0 &gt; 0">
          <value>TOOLTIP</value>
        </exp>
        <pv_name>loc://A-BOOLEAN</pv_name>
      </rule>
    </rules>
    <tooltip>$(text)</tooltip>
  </widget>
  <widget type="textupdate" version="2.0.0">
    <name>Text Update_1</name>
    <pv_name>loc://A-BOOLEAN(0)</pv_name>
    <x>140</x>
    <y>70</y>
    <rules>
      <rule name="New Rule" prop_id="tooltip" out_exp="false">
        <exp bool_exp="pvInt0 == 0">
          <value>NO TOOLTIP</value>
        </exp>
        <exp bool_exp="pvInt0 &gt; 0">
          <value>TOOLTIP</value>
        </exp>
        <pv_name>loc://A-BOOLEAN</pv_name>
      </rule>
    </rules>
  </widget>
</display>

It seems like

else:
  widget.setPropertyValue('tooltip', "$(pv_name)
  $(pv_value)")

is coming from the original default value of the Tooltip property.

kasemir commented 6 years ago

The else part of the rule sets the property to the current value of the property. In your case, the current value seems to be the default value, which is "$(pv_name)\n$(pv_value)" for a tooltip property.

So the rule-to-script code definitely needs an update to handle string values, where newlines need to be properly escaped. Not sure if that completely solves the issue, there may be more to do.

claudio-rosati commented 6 years ago

It seems reasonable.

kasemir commented 6 years ago

Try again with the latest from master. The string literals generated for Jython now escape the newline, and the NPE is also gone.

claudio-rosati commented 6 years ago

@kasemir I've tried and everything is working. Only the following problem still exists (is it really a problem?):

2018-03-20 08:51:49.869 WARNING [Thread 1] org.csstudio.display.builder.model.WidgetProperty (getPath) - Cannot determine path to text for Widget 'Label_1' (label)

If that is not a problem, please feel free to close the issue.

kasemir commented 6 years ago

Try now, the warning should be gone

claudio-rosati commented 6 years ago

Works fine. Thank you.