BDX transfer support for Python tests #34821
Conversation
…e transfer delegate.
Also add the transfer obtained context to the C++ methods relating to expecting transfers.
…a BDX transfer. Also run the python BDX initialisation.
Also ignore the BDX transfer server implementation that only handles diagnostic logs.
…ata. * Call Responder::PrepareForTransfer from BdxTransfer. * Correctly schedule satisfying the future on the event loop. * Use the real property to determine if a PyChipError was a success.
Also don't ignore all messages after the init.
…he way one would expect.
pullapprove
bot
requested review from
rcasallas-silabs,
ReneJosefsen,
robszewczyk,
saurabhst,
selissia,
sharadb-amazon,
tcarmelveilleux,
tecimovic,
tehampson,
tima-q,
tobiasgraf,
turon,
vivien-apple,
wiba-nordic,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
* remove a check that was inadvertently kept. * print a log message when something that shouldn't happen inevitably does. * use user_params to get the end user support log test parameter.
|
|
||
|
|
||
| class BdxTransfer: | ||
| def __init__(self, bdx_transfer: c_void_p, init_message: InitMessage, data=None): |
There was a problem hiding this comment.
Can you add a type annotation for data? Is it expecting bytes? or string? or something else entirely
| eventLoop = asyncio.get_running_loop() | ||
| future = eventLoop.create_future() | ||
|
|
||
| if self._data != None: |
There was a problem hiding this comment.
| if self._data != None: | |
| if self._data is not None: |
I think Or not self._data if data can't be a thing that evaluates to false. Not sure on the data type or whether an empty data is viable.
| OnTransferCompletedCallback gOnTransferCompletedCallback = nullptr; | ||
|
|
||
| // The information for a single transfer. | ||
| struct TransferData |
There was a problem hiding this comment.
The name of this struct is a bit odd because it's not the data for transfer, its the information about the transfer. Can this be TranferInfo or some other such thing
| if (gOnTransferObtainedCallback && transferData) | ||
| { | ||
| gOnTransferObtainedCallback(transferData->OnTransferObtainedContext, ToPyChipError(result), nullptr, | ||
| static_cast<bdx::TransferControlFlags>(0), 0, 0, 0, nullptr, 0, nullptr, 0); |
| // The information for a single transfer. | ||
| struct TransferData | ||
| { | ||
| bdx::BdxTransfer * Transfer = nullptr; |
There was a problem hiding this comment.
I think I've tracked down all the pieces, and it seems correct, but would you mind adding some documentation here about what owns which pieces of this transfer and what the expected lifefimes are? The callbacks are python, it looks like the TransferData is owned by the TransferMap (owned by ...? But referenced by the delegate?) Transfers are owned by the transfer server? Does the transfer server also own the data or is that owned externally? Does that change for receive/send?
Add support for BDX transfers to the Python tests.
This allows Python tests to wait for BDX transfers, inspect the init message's details, accept or reject the transfer, and send or receive data. This assists with writing tests for the OTA requestor and diagnostic logs clusters that involve BDX transfers. The code takes some inspiration from the existing BDX transfer server code in src/protocols/bdx, and I think it's worth following up at some point to expand the capabilities of that code to reduce the amount of code specific to the Python controller code introduced in this change.
Included is a test that uses the diagnostic logs cluster on the all clusters app to initiate and complete a BDX transfer.