Skip to content
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

Merged
merged 5 commits into from Dec 6, 2023
Merged

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Dec 6, 2023

PR #12467 introduced auto function return value that wasn't liked:

rs-dds-sniffer.cpp:52:15: error: 'print_guid' function uses 'auto' type specifier without trailing return type

Only on jammy debian creation...

Added some small changes in manual usage of devices.py script, to avoid annoying wait times and compound behavior.

@maloel maloel requested a review from Nir-Az December 6, 2023 09:32
@@ -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:
Copy link
Collaborator

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..

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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...?

Copy link
Collaborator

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

Copy link
Collaborator Author

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)

Copy link
Collaborator Author

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?

Copy link
Collaborator

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 :-)

@maloel maloel merged commit 2c0be46 into IntelRealSense:development Dec 6, 2023
16 of 17 checks passed
@maloel maloel deleted the acroname branch December 6, 2023 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants