Tickets/dm 48022 #254
Tickets/dm 48022 #254
Conversation
| @@ -75,6 +75,10 @@ def get_schema(cls): | |||
| description: Ignore the m1m3 component? | |||
| type: boolean | |||
| default: false | |||
There was a problem hiding this comment.
You might want to remove the default value here or it will always trigger the deprecation warning. Also, please add a note in the description that this is going to be deprecated.
| @@ -107,8 +115,3 @@ async def run(self): | |||
| elapsed_time = end_time - start_time | |||
|
|
|||
| self.log.info(f"Homing both axes took {elapsed_time:.2f} seconds") | |||
|
|
|||
| if not self.ignore_m1m3: | |||
There was a problem hiding this comment.
if you disabled it, you need to enable it back here.
There was a problem hiding this comment.
Not sure I follow. ignore_m1m3 it's basically removed. So there is no "ignore_m1m3" property.
There was a problem hiding this comment.
but we added the disable_m1m3_force_balance option....
There was a problem hiding this comment.
aah got it! of course we disable it on the CSC not the script!
In that case we want to keep the warn_wait? See that originally we had a asyncio.sleep just b4 enabling the balance system.
| self.home_both_axes_timeout = 300.0 # timeout to home both MTMount axes. | ||
|
|
||
| self.ignore_m1m3 = False | ||
| self.warn_wait = 10.0 |
There was a problem hiding this comment.
I think you can remove this attribute now.
There was a problem hiding this comment.
It is removed already, see the "-"
| self.ignore_m1m3 = config.ignore_m1m3 | ||
|
|
||
| if hasattr(config, "ignore_m1m3"): | ||
| warnings.warn( |
There was a problem hiding this comment.
this warning will be printed to stdout (or stderr, I am not sure) but won't be logged. Please, add a self.log.warning call here as well.
doc/news/DM-48022.feature.rst
Outdated
| @@ -0,0 +1,6 @@ | |||
| Deprecate `ignore_m1m3` property. | |||
There was a problem hiding this comment.
There's a "removal" type you can probably use to document this one.
Please, split both of these notes in 2 different files.
doc/news/DM-48022.feature.rst
Outdated
| @@ -0,0 +1,6 @@ | |||
| Deprecate `ignore_m1m3` property. | |||
| Added new property `disable_m1m3_force_balance` with default `false`. | |||
| Maintains the ability to disable the M1M3 balance system, in case | |||
There was a problem hiding this comment.
I think you can remove this space here before each line. Also, we use "semantic line breaking" so please, make sure each phrase is in one line.
need to disable balance system when homing. Deprecate ignore_m1m3 added disable_m1m3_force_balance property - Deprecate `ignore_m1m3` property. - Added new property `disable_m1m3_force_balance` with default `false`.
iglesu
force-pushed
the
tickets/DM-48022
branch
from
87c48c2 to
f046ad6
Compare
https://rubinobs.atlassian.net/browse/DM-48022
Deprecate ignore_m1m3 added disable_m1m3_force_balance property