add multiple hubs support - #13978
Conversation
| run_time_stopwatch = Stopwatch() | ||
| run_time_threshold = 5 if is_dds else 2 # currently, DDS devices take longer time to complete rs_enumerate_devices | ||
| p = subprocess.run( [rs_enumerate_devices], | ||
| p = subprocess.run( [rs_enumerate_devices, "--no-dds" if not is_dds else ""], |
There was a problem hiding this comment.
Maybe we should somehow align the test for DDS devices also somehow?
It's a good test for D555 too
Please add it to the adding tests for D555 ticket
There was a problem hiding this comment.
This test is already aligned for DDS devices and passes there
There was a problem hiding this comment.
But once you turn on DDS is_dds will always be true no?
There was a problem hiding this comment.
no, is_dds is true only if the connection type is DDS, for USB it should still be False
| return self.hub_port_to_virtual_port[(name, int(port))] | ||
| return None | ||
|
|
||
| # recycle_ports inherited; uses ports(), disable_ports(), enable_ports() |
There was a problem hiding this comment.
What does this comment means?
There was a problem hiding this comment.
Old comment, I will remove
| return unifi_hub | ||
| active_hubs.append(unifi_hub) | ||
|
|
||
| if len(active_hubs) > 1: |
There was a problem hiding this comment.
Why not working always with the combined hub?
There was a problem hiding this comment.
Just to simplify when it's not needed - if there's one hub we can do:
devices -> hub
instead of
devices -> combined hub -> hub
| self._is_dds = False | ||
|
|
||
| self._usb_location = None | ||
| self._location = None # might be either usb location or mac address |
There was a problem hiding this comment.
currently it will hold None, but will fixed as I said on the other comment
| try: | ||
| self._usb_location = _get_usb_location(self._physical_port) | ||
| if self._is_dds: | ||
| self._location = get_mac_address(dev) |
There was a problem hiding this comment.
GMSL handling?
Today we always get this error
Maybe you an handle it?
There was a problem hiding this comment.
Sure, I'll call _get_usb_location(self._physical_port) only if the connection type is "USB", otherwise I'll print it's GMSL
| # Get all devices, and store by serial-number | ||
| global _device_by_sn, _context, _port_to_sn | ||
| settings = {} | ||
| settings = {'dds' : { 'enabled' : True }} # explicitly enable dds in case there's an issue with the config file |
There was a problem hiding this comment.
What will happen in the private branch when BUILD_WITH_DDS=OFF?
There was a problem hiding this comment.
Tried checking locally with this flag off and seeing no issue, so should be fine there too
| print( ' --enable Enable all ports' ) | ||
| print( ' --disable Disable all ports' ) | ||
| print( ' --recycle Recycle all ports' ) | ||
| print( ' --enable=<ports> Enable <ports>' ) |
There was a problem hiding this comment.
Maybe add an example how you need ports to be transferred?
With spaces? commas?
There was a problem hiding this comment.
print( ' <ports> is a comma-separated list of port numbers, or empty for all ports' )
a few lines after this, need to add an example here too?
There was a problem hiding this comment.
Missed it, maybe shift it up after the last argument that accept ports? your call
Nir-Az
left a comment
There was a problem hiding this comment.
Looks great overall, nice work.
I approve but added a few comments for you to consider.
Once you update all comments we can merge it
Tracked on: [LRS-1223]