Configure impl #154
Configure impl #154
Conversation
windows/devices/uwb/include/windows/devices/uwb/UwbCxAdapter.hxx
Outdated
Show resolved
Hide resolved
| } | ||
| if (deviceType == DeviceType::Controller) { | ||
| if (mode == uwb::UwbMacAddressType::Short) { | ||
| return config.GetControleeShortMacAddress(); // TODO this should reflect the possibility for multiple peers |
There was a problem hiding this comment.
This might not be necessary. If our current understanding is accurate, then we'll expect many UwbSessionData (with UwbConfiguration in them) to be provided via OOB, so we'd call UwbSession::Configure for each of them after converting them with this code, and each of them should only contain a single address for the controllee. Eg.
DeviceType deviceType = DeviceType::Controller; // from OOB "profile"
UwbSession uwbSession = uwbDevice.CreateSession(deviceType);
foreach (UwbSessionData from OOB /* = N */) {
auto applicationConfigurationParameters = GetUciConfigParams(UwbSessionData.uwbConfiguration, deviceType);
uwbSession.Configure(uwbSessionData.sessionId, std::move(applicationConfigurationParameters));
}
// uwbSession is now configured for 'N' sets of parameters for 'N' controlees
I think that when the host is a controller, it will set a lot of the application configuration parameters itself, whereas when the host is a controlee, it gets most of the parameters from the controller, and there will likely only be one UwbSessionData from OOB.
There was a problem hiding this comment.
Now that I think about it, it might not make sense for UwbSession to be constructed and also not call SessionInitialize at the same time. This is because currently the interface does allow the sessionId to change after Configure has been called (via multiple calls of Configure), while what we really want is UwbSession to represent one unique session. We can address this in another PR
There was a problem hiding this comment.
I was thinking something similar, where the caller could check the session id prior to calling Configure.
There was a problem hiding this comment.
UCI has a parameter named NUMBER_OF_CONTROLEES. Does this mean that in our implementation it will always be set to 1?
| @@ -549,7 +549,7 @@ using UwbApplicationConfigurationParameterValue = std::variant< | |||
| RangingRoundUsage, // RANGING_ROUND_USAGE, tag0x01, size 1 | |||
| RangingMode, // RANGING_TIME_STRUCT, tag 0x1A, size 1 | |||
| RangingRoundControl, // RANGING_ROUND_CONTROL, tag 0x0C, size 1, | |||
| ResultReportConfiguration, // RESULT_REPORT_CONFIG, tag 0x2E, size 1 | |||
| ResultReportConfiguration, // RESULT_REPORT_CONFIG, tag 0x2E, size 1 // TODO this should really be a set | |||
There was a problem hiding this comment.
Might as well fix this now so we can close this chapter.
There was a problem hiding this comment.
I would prefer to do this in another PR (keeps this one small)
Type
Goals
This PR changes the interface of UwbSession::Configure to take in the sessionId and a vector of UwbApplicationConfigurationParameter because those are what UCI defines as inputs to the driver for the IOCTL.
This PR also fixes some minor documentation issues with the UwbDeviceConnector
Also the UwbConfiguration::uwbConfigurationAvailable boolean was deemed unnecessary because you can just check the output of UwbConfiguration::GetValueMap(), and so it was removed.
Future Work
We need to implement the translation function between the OOB app config params to the UCI config params. I have the function declared as
and it is declared in lib/uwb/protocols/fira/UwbSession.hxx
Checklist
allcompiles cleanly.