epics-base / p4p

Python bindings for the PVAccess network client and server.
BSD 3-Clause "New" or "Revised" License
24 stars 37 forks source link

KeyError: "no such member 'display.description' in 'display.description'" for NTScalar string types #113

Open kathryn-baker opened 1 year ago

kathryn-baker commented 1 year ago

Hi there, I am trying to create a string PV with a description and I am using the same construction method as I have for similar Double PVs. However, when passing type 's' to the NTScalar class, I get the following error:

pv = SharedPV(nt=NTScalar('s', display=True), initial={'value':'test'})  # no error
pv = SharedPV(nt=NTScalar('d', display=True), initial={'value': 0.0, 'display.description':'test'})  # no error
pv = SharedPV(nt=NTScalar('s', display=True), initial={'value':'test', 'display.description':'test'})

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/p4p/server/thread.py", line 76, in __init__
    _SharedPV.__init__(self, **kws)
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 133, in __init__
    self.open(initial, nt=nt, wrap=wrap, unwrap=unwrap, **kws)
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 150, in open
    _SharedPV.open(self, self._wrap(value, **kws))
  File "/usr/local/lib/python3.10/site-packages/p4p/nt/scalar.py", line 211, in wrap
    value = self.Value(self.type, value)
  File "src/p4p/_p4p.pyx", line 150, in p4p._p4p._Value.__init__
KeyError: "no such member 'display.description' in 'display.description'"

Is this the expected behaviour for string PVs?

I am using p4p version 4.1.5 with python 3.10.12

mdavidsaver commented 1 year ago

Should be fixed by 2797fd3f483a6d0d67336cb89d060eadbe21963e (in 4.1.8)

kathryn-baker commented 1 year ago

Thank you for such a quick response!

I've just upgraded to p4p==4.1.8 and I'm now able to create the PV objects using the code above. However, when I try to access the pv.current() value, I get the following error in 4.1.8:

>>> pv = SharedPV(nt=NTScalar('d', display=True), initial={'value': 0.0, 'display.description':'test'})
>>> pv.current().raw.todict()
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 182, in current
    return self._unwrap()
TypeError: NTScalar.unwrap() missing 1 required positional argument: 'value'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 184, in current
    raise ValueError("Unable to unwrap %r with %r"%(V, self._unwrap))
ValueError: Unable to unwrap Value(id:epics:nt/NTScalar:1.0, 0.0) with <bound method NTScalar.unwrap of <class 'p4p.nt.scalar.NTScalar'

The same error occurs without display=True as well.

I've checked and the error does not exist in version 4.1.7:

>>>pv = SharedPV(nt=NTScalar('d', display=True), initial={'value': 0.0, 'display.description':'test'})
>>>pv.current().raw.todict()
{'value': 0.0, 'alarm': {'severity': 0, 'status': 0, 'message': ''}, 'timeStamp': {'secondsPastEpoch': 0, 'nanoseconds': 0, 'userTag': 0}, 'display': {'limitLow': 0.0, 'limitHigh': 0.0, 'description': 'test', 'format': '', 'units': ''}}
juanfem commented 1 year ago

This seems to be a regression. @mdavidsaver I think the issue is here: https://github.com/mdavidsaver/p4p/commit/23e26ca005c42229f8df42052df04bee8a3d1b58#r122603477

mdavidsaver commented 1 year ago

@juanfem There is. 2d312a193ce60cf70708e361d760711885979502 should fix this. I have pushed out p4p 4.1.9 with this change.

kathryn-baker commented 1 year ago

Thanks!

Using 4.1.9 I'm able to create the string PV with a description as I need. I'm not expecting to be able to specify the format or units with a string PV because they don't really need it but I get the same error as before when I pass a dictionary that contains the format and units, even if they're blank.

For my application this is fine but I thought I'd check whether this is the intention?

>>> pv = SharedPV(nt=NTScalar('s', display=True), initial={'value': 0.0, 'display.description':'test'})  # works correctly
>>> pv = SharedPV(nt=NTScalar('s', display=True), initial={'value': 0.0, 'display.description':'test', 'display.format': ''})  # error
Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 158, in open
    V = self._wrap(value, **kws)
  File "/usr/local/lib/python3.10/site-packages/p4p/nt/scalar.py", line 218, in wrap
    value = self.Value(self.type, value)
  File "src/p4p/_p4p.pyx", line 150, in p4p._p4p._Value.__init__
KeyError: "no such member 'display.format' in 'display.format'"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.10/site-packages/p4p/server/thread.py", line 76, in __init__
    _SharedPV.__init__(self, **kws)
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 140, in __init__
    self.open(initial, nt=nt, wrap=wrap, unwrap=unwrap, **kws)
  File "/usr/local/lib/python3.10/site-packages/p4p/server/raw.py", line 160, in open
    raise ValueError("Unable to wrap %r with %r and %r"%(value, self._wrap, kws))
ValueError: Unable to wrap {'value': 0.0, 'display.description': 'test', 'display.format': ''} with <bound method NTScalar.wrap of <p4p.nt.scalar.NTScalar object at 0x7fbb487b1c60>> and {}
mdavidsaver commented 1 year ago

... For my application this is fine but I thought I'd check whether this is the intention?

Yes. In part because display.format does not make sense for a string. It is also considered deprecated in favor of display.precision and display.form. Which reminds me that this needs to go into P4P. Ideally while also removing some redundancy with the equivalent code in PVXS.

mdavidsaver commented 1 year ago

... display.precision and display.form. Which reminds me that this needs to go into P4P.

360c131d9cdac6c1fc8264983a038722383f41eb adds an argument form=True which can be used along with display=True. Although the display.form.choices list is not (yet?) populated automatically.