-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix auto return value error grrrrr #12491
Conversation
unit-tests/py/rspy/devices.py
Outdated
@@ -219,8 +219,9 @@ def query( monitor_changes = True, hub_reset = False ): | |||
if not acroname.hub: | |||
acroname.connect( hub_reset ) # MAY THROW! | |||
|
|||
acroname.disable_ports( sleep_on_change = 5 ) | |||
acroname.enable_ports( sleep_on_change = MAX_ENUMERATION_TIME ) | |||
if monitor_changes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does monitor_changes
affect the flow of disabling and enabling the devices?
I know it doesn't change the main flow but it looks not related..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on by default, and run-unit-tests leaves it on.
I.e., this affects only using devices.py thru the command-line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted it to NOT disable and enable devices when connecting. That's what this does. Leaves the state the same as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can also add another argument...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am just saying it looks like thus parameter is abused agaisnt it's name
param monitor_changes: If True, devices will update dynamically as they are removed/added
now it will also do other stuff..your call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to recycle_ports
argument (default is True)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use hub_reset
, maybe we should disable recycle_ports
, then? Aren't they already recycled if the hub is reset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In most cases you are right.
If we take into consideration that all ports were turn off on last run then we start off, and acroname reset turn all on IMO.
But if last run did not turn off then there is only a very quick power cycle (if any) to the caneras and we can get weird behaviors.
We can reduce the time after disabling from 5 to 2 for sure :-)
PR #12467 introduced
auto
function return value that wasn't liked:Only on jammy debian creation...
Added some small changes in manual usage of
devices.py
script, to avoid annoying wait times and compound behavior.