epics-base / p4p

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

add ntbool support to NTScalar automatic value unwrapping #37

Closed ddamiani closed 5 years ago

ddamiani commented 5 years ago

The default NTScalar unwrapping throws an exception when getting a boolean PV. There isn't an ntbool I assume because bool can't be subclassed? I add an ntbool class that is a subclass of int, but only can have a value of 0 or 1. It's repr also mimics the built-in boolean.

Example of it in action from the client-side

In [10]: ctxt.get('demo:pv:n2')                                                                                                                               
Out[10]: True
In [11]: print(ctxt.get('demo:pv:n2'))                                                                                                                        
Thu Apr 11 09:51:48 2019 True
In [12]: ctxt.get('demo:pv:n2').timestamp                                                                                                                     
Out[12]: 1555001508.1798918
In [13]: type(ctxt.get('demo:pv:n2'))                                                                                                                      
Out[13]: p4p.nt.scalar.ntbool

Also includes a small edit to the str method of ntwrappercommon so it will use the repr of the nt* class if it has one (like this ntbool does).

klauer commented 5 years ago

That (admittedly nice) boolean repr may make users think it's safe to do is True or is not False-style comparisons, which will not work.

mdavidsaver commented 5 years ago

@ddamiani Sorry, I pushed out 3.3.0 this morning without checking for other open PRs.

ddamiani commented 5 years ago

Maybe it's safer then to have it return 0 or 1 as the repr, so no one tries doing is True or is False?

mdavidsaver commented 5 years ago

Merged with an additional change 313bb75724a25fec908c29805b990341cd765681 to print "true" or "false", which will hopefully be less ambiguous.