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
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tools/dds/dds-sniffer/rs-dds-sniffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ constexpr uint8_t GUID_PROCESS_LOCATION = 4;

static eprosima::fastrtps::rtps::GuidPrefix_t std_prefix;

static inline auto print_guid( realdds::dds_guid const & guid )
static inline realdds::print_guid print_guid( realdds::dds_guid const & guid )
{
return realdds::print_guid( guid, std_prefix );
}
Expand Down
22 changes: 15 additions & 7 deletions unit-tests/py/rspy/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def map_unknown_ports():
log.debug_unindent()


def query( monitor_changes = True, hub_reset = False ):
def query( monitor_changes=True, hub_reset=False ):
"""
Start a new LRS context, and collect all devices
:param monitor_changes: If True, devices will update dynamically as they are removed/added
Expand All @@ -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 :-)

acroname.disable_ports( sleep_on_change = 5 )
acroname.enable_ports( sleep_on_change = MAX_ENUMERATION_TIME )

if platform.system() == 'Linux':
global _acroname_hubs
Expand Down Expand Up @@ -751,7 +752,7 @@ def _get_port_by_loc( usb_location ):
import os, sys, getopt
try:
opts,args = getopt.getopt( sys.argv[1:], '',
longopts = [ 'help', 'recycle', 'all', 'list', 'port=', 'ports' ])
longopts = [ 'help', 'recycle', 'all', 'none', 'list', 'port=', 'ports' ])
except getopt.GetoptError as err:
print( '-F-', err ) # something like "option -a not recognized"
usage()
Expand Down Expand Up @@ -785,19 +786,26 @@ def get_phys_port(dev):
ports = [int(port) for port in str_ports if port.isnumeric() and int(port) in all_ports]
if len(ports) != len(str_ports):
log.f( 'Invalid ports', str_ports )
acroname.enable_ports( ports, disable_other_ports = True, sleep_on_change = MAX_ENUMERATION_TIME )
acroname.enable_ports( ports, disable_other_ports=False )
action = 'none'
elif opt in ('--ports'):
printer = get_phys_port
elif opt in ('--all'):
if not acroname:
log.f( 'No acroname available' )
acroname.enable_ports( sleep_on_change = MAX_ENUMERATION_TIME )
acroname.enable_ports()
action = 'none'
elif opt in ('--none'):
if not acroname:
log.f( 'No acroname available' )
acroname.disable_ports()
action = 'none'
elif opt in ('--recycle'):
action = 'recycle'
else:
usage()
if action == 'list':
query()
query( monitor_changes=False )
for sn in all():
device = get( sn )
print( '{port} {name:30} {sn:20} {handle}'.format(
Expand Down
Loading