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

Separate driver and service testing functionality in nocli #169

Merged
merged 6 commits into from
Mar 24, 2023

Conversation

corbin-phipps
Copy link
Contributor

@corbin-phipps corbin-phipps commented Mar 24, 2023

Type

  • Bug fix
  • Feature addition
  • Feature update
  • Breaking change
  • Non-functional change
  • Documentation
  • Infrastructure

Goals

This PR separates the driver testing functionality (under sub-command 'uwb') from service testing functionality (under new sub-command 'service'). This is necessary for configuring ranging sessions, because users testing drivers will need to set UCI app config params whereas users testing service code will need to set OOB params.

Technical Details

  • Created new top-level sub-command calls 'service'
  • Restored original 'range start' code and moved under 'service' (nocli.exe service range start)
  • Modified 'range start' code for driver testing and moved under 'uwb' (nocli.exe uwb range start)
  • Added structures under NearObjectCliData.hxx for setting mandatory app config params for driver testing
  • Added support for unordered_set in a conversion function in UwbCxAdapterDdiLrp.cxx
  • Fixed implementation of UwbDeviceConnector::SetApplicationConfigurationParameters to use new wrapper for UWB_SET_APP_CONFIG_PARAMS

Test Results

Ran nocli tool for driver testing ('uwb' sub-command) with newest TestClient driver and can successfully invoke necessary IOCTLs for starting a ranging session.

Reviewer Focus

Note: The current code in HandleDriverStartRanging tries to get the app config params after configuring a session. Because the TestClient driver does not support this (yet!), I had to comment these few lines out in order for nocli to work.

Future Work

  • Need to ensure that stopping the ranging session will trigger StopRanging and SessionDeinit.
  • Add rest of UCI app config params support in NearObjectCli.cxx. Currently, only the mandatory app config params are supported
  • The code for 'uwb range start' could be improved. For example, the current code manually pushes each of the mandatory app config params into a std::vector
  • Multicast support (more than one destination MAC address / controlee).

Checklist

  • Build target all compiles cleanly.
  • clang-format and clang-tidy deltas produced no new output.
  • Newly added functions include doxygen-style comment block.

@corbin-phipps corbin-phipps requested a review from a team as a code owner March 24, 2023 17:15
@corbin-phipps
Copy link
Contributor Author

Current CLI output (with GetAppConfigParams code commented out of HandleDriverStartRanging):
image

tools/cli/include/nearobject/cli/NearObjectCliData.hxx Outdated Show resolved Hide resolved
windows/devices/uwb/UwbCxAdapterDdiLrp.cxx Show resolved Hide resolved
tools/cli/NearObjectCli.cxx Outdated Show resolved Hide resolved
tools/cli/NearObjectCli.cxx Outdated Show resolved Hide resolved
tools/cli/NearObjectCli.cxx Outdated Show resolved Hide resolved
Copy link
Contributor

@abeltrano abeltrano left a comment

Choose a reason for hiding this comment

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

Love it

@corbin-phipps corbin-phipps merged commit a302005 into develop Mar 24, 2023
@corbin-phipps corbin-phipps deleted the user/corbinphipps/nocli-separate-oob-uci branch March 24, 2023 21:06
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.

2 participants