tango-controls / pytango

This project was moved to gitlab.com
http://pytango.rtfd.io
54 stars 44 forks source link

Fix read/write/is_allowed not called for dynamic attribute in async mode server #337

Closed stanislaw55 closed 2 years ago

stanislaw55 commented 4 years ago

Hi, this is PR with fix for #173.

Now it is a draft with (failing) tests only

stanislaw55 commented 4 years ago

@ajoubertza, I have a problem with Attr class. It needs to get in its constructor one of tango types (DevFloat, DevBoolean) but types that I get from typed_values pytest fixture, are pure Python ones and tests are failing because of that

stanislaw55 commented 4 years ago

Another thing, I wanted to test is_allowed method but I'm not sure how to do it. MY first idea was to use state fixture and bind output of is_allowed to state of the device but I'm not sure if that's good idea

ajoubertza commented 4 years ago

Thanks for starting on this!

@ajoubertza, I have a problem with Attr class. It needs to get in its constructor one of tango types (DevFloat, DevBoolean) but types that I get from typed_values pytest fixture, are pure Python ones and tests are failing because of that

I see there are a few conversion method in tango.utils, although the one I'd prefer to use tango.utils.get_tango_type() fails. So as a workaround, there is another implementation in the device_proxy module (and same again in pipe module) that can be used in the meantime. Looks like we need to rationalise all these conversion methods!

In [1]: import tango, tango.test_utils                                                                                                                                

In [2]: [tango.device_proxy.__get_tango_type(t) for t in tango.test_utils.TYPED_VALUES]                                                                               
Out[2]: 
[tango._tango.CmdArgType.DevLong64,
 tango._tango.CmdArgType.DevDouble,
 tango._tango.CmdArgType.DevString,
 tango._tango.CmdArgType.DevBoolean,
 tango._tango.CmdArgType.DevVarLong64Array,
 tango._tango.CmdArgType.DevVarDoubleArray,
 tango._tango.CmdArgType.DevVarStringArray,
 tango._tango.CmdArgType.DevVarBooleanArray]
ajoubertza commented 4 years ago

Another thing, I wanted to test is_allowed method but I'm not sure how to do it. MY first idea was to use state fixture and bind output of is_allowed to state of the device but I'm not sure if that's good idea

Maybe just set some variables if you hit the is_allowed method, and the actual attribute read/write methods. Then assert on those? Could try a class variable like here.

stanislaw55 commented 4 years ago

Hi @ajoubertza Thanks for help! Now I hope tests are good enough to catch proper bug

ajoubertza commented 4 years ago

Thanks for the updates @stanislaw55 - its looking better. I've resolved the comments you have addressed.