Skip to content

devices.py --list now shows DDS devices; fix error - #13703

Merged
maloel merged 1 commit into
realsenseai:developmentfrom
maloel:devices-eth
Jan 28, 2025
Merged

devices.py --list now shows DDS devices; fix error#13703
maloel merged 1 commit into
realsenseai:developmentfrom
maloel:devices-eth

Conversation

@maloel

@maloel maloel commented Jan 23, 2025

Copy link
Copy Markdown
Contributor

Kept the old behavior, where DDS devices are not queried by default from run-unit-tests...
But now, at least from the command-line (which I use), you can do devices --list and you'll see all devices including eth and it'll show you their ports.

Also fixed a bug with eth devices port lookup.

@maloel maloel added ci CI-related: does not change library behavior dds labels Jan 23, 2025
@maloel
maloel requested a review from Nir-Az January 23, 2025 15:10
@Nir-Az

Nir-Az commented Jan 26, 2025

Copy link
Copy Markdown
Collaborator

Since @AviaAv is working on the same code that we can add D555 to LibCI best that we do a cross review :)

@Nir-Az
Nir-Az requested a review from AviaAv January 26, 2025 15:41
Comment thread unit-tests/py/rspy/devices.py Outdated
_context = rs.context( { 'dds': False } )
settings = {}
if disable_dds:
settings['dds'] = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to do: settings['dds'] = { 'enabled' : False}
Both options work for disabling DDS, but when we use settings['dds'] = True we still won't see DDS devices, so best to be consistent about it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a shortcut -- if dds is not an object, then everything inside the missing object gets the default value and enabled becomes False.
But OK, will do.



def query( monitor_changes=True, hub_reset=False, recycle_ports=True ):
def query( monitor_changes=True, hub_reset=False, recycle_ports=True, disable_dds=True ):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't we want to show dds devices by default? disable_dds=False?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to change existing behavior

if action == 'list':
query( monitor_changes=False, recycle_ports=False, hub_reset=False )
query( monitor_changes=False, recycle_ports=False, hub_reset=False, disable_dds=False )
map_unknown_ports()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but can you please explain why map_unknown_ports() is needed here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because otherwise I wouldn't know which port contains the ethernet device. :)

@AviaAv

AviaAv commented Jan 28, 2025

Copy link
Copy Markdown
Contributor

LGTM, some small comments

@maloel
maloel merged commit fc3ae5c into realsenseai:development Jan 28, 2025
@maloel
maloel deleted the devices-eth branch January 28, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI-related: does not change library behavior dds

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants