Skip to content

Commit

Permalink
Fix some of the minor TODOs.
Browse files Browse the repository at this point in the history
  • Loading branch information
harimau-qirex committed Sep 6, 2024
1 parent 6ba2bfe commit 2403120
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 23 deletions.
19 changes: 17 additions & 2 deletions src/controller/python/chip/bdx/bdx-transfer-manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ BdxTransfer * BdxTransferManager::Allocate()

BdxTransfer * result = mTransferPool.CreateObject();
VerifyOrReturn(result != nullptr);
// TODO: This needs to intercept the delegate calls so it can free the transfer after it completes.
result->SetDelegate(mBdxTransferDelegate);
result->SetDelegate(this);

--mExpectedTransfers;
return result;
Expand All @@ -58,5 +57,21 @@ void BdxTransferManager::Release(BdxTransfer * bdxTransfer)
mTransferPool.ReleaseObject(bdxTransfer);
}

void BdxTransferManager::InitMessageReceived(BdxTransfer * transfer, TransferSession::TransferInitData init_data)
{
mBdxTransferDelegate->InitMessageReceived(transfer, init_data);
}

void BdxTransferManager::DataReceived(BdxTransfer * transfer, const ByteSpan & block)
{
mBdxTransferDelegate->DataReceived(transfer, block);
}

void BdxTransferManager::TransferCompleted(BdxTransfer * transfer, CHIP_ERROR result)
{
mBdxTransferDelegate->TransferCompleted(transfer, result);
mTransferPool.ReleaseObject(transfer);
}

} // namespace bdx
} // namespace chip
6 changes: 5 additions & 1 deletion src/controller/python/chip/bdx/bdx-transfer-manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace bdx {

// This class implements the pool interface used to allocate BdxTransfer objects. It keeps track of the number of transfers
// that are expected to be created and only allocates a BdxTransfer object if a transfer is expected.
class BdxTransferManager : public BdxTransferPool
class BdxTransferManager : public BdxTransferPool, public BdxTransfer::Delegate
{
public:
BdxTransferManager(BdxTransfer::Delegate * bdxTransferDelegate);
Expand All @@ -41,6 +41,10 @@ class BdxTransferManager : public BdxTransferPool
BdxTransfer * Allocate() override;
void Release(BdxTransfer * bdxTransfer) override;

void InitMessageReceived(BdxTransfer * transfer, TransferSession::TransferInitData init_data) override;
void DataReceived(BdxTransfer * transfer, const ByteSpan & block) override;
void TransferCompleted(BdxTransfer * transfer, CHIP_ERROR result) override;

private:
ObjectPool<BdxTransfer, 2> mTransferPool;
BdxTransfer::Delegate * mBdxTransferDelegate = nullptr;
Expand Down
31 changes: 15 additions & 16 deletions src/controller/python/chip/bdx/bdx-transfer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ CHIP_ERROR BdxTransfer::AcceptSend()
VerifyOrReturnError(mAwaitingAccept, CHIP_ERROR_INCORRECT_STATE);
mAwaitingAccept = false;

TransferSession::TransferAcceptData accept_data;
accept_data.ControlMode = mInitData.TransferCtlFlags; // TODO: Is this value correct?
accept_data.MaxBlockSize = mInitData.MaxBlockSize;
// TODO: Check the acceptable control modes for a Diagnostic Logs cluster transfer.
return mTransfer.AcceptTransfer(accept_data);
TransferSession::TransferAcceptData acceptData;
acceptData.ControlMode = TransferControlFlags::kSenderDrive;
acceptData.MaxBlockSize = mTransfer.GetTransferBlockSize();
acceptData.StartOffset = mTransfer.GetStartOffset();
acceptData.Length = mTransfer.GetTransferLength();
return mTransfer.AcceptTransfer(acceptData);
}

CHIP_ERROR BdxTransfer::AcceptReceive(const ByteSpan data_to_send)
CHIP_ERROR BdxTransfer::AcceptReceive(const ByteSpan & data_to_send)
{
VerifyOrReturnError(mAwaitingAccept, CHIP_ERROR_INCORRECT_STATE);
mAwaitingAccept = false;
Expand All @@ -60,10 +61,12 @@ CHIP_ERROR BdxTransfer::AcceptReceive(const ByteSpan data_to_send)
memcpy(mData, data_to_send.data(), data_to_send.size());
mDataCount = data_to_send.size();

TransferSession::TransferAcceptData accept_data;
accept_data.ControlMode = mInitData.TransferCtlFlags; // TODO: Is this value correct?
accept_data.MaxBlockSize = mInitData.MaxBlockSize;
return mTransfer.AcceptTransfer(accept_data);
TransferSession::TransferAcceptData acceptData;
acceptData.ControlMode = TransferControlFlags::kReceiverDrive;
acceptData.MaxBlockSize = mTransfer.GetTransferBlockSize();
acceptData.StartOffset = mTransfer.GetStartOffset();
acceptData.Length = mTransfer.GetTransferLength();
return mTransfer.AcceptTransfer(acceptData);
}

CHIP_ERROR BdxTransfer::Reject()
Expand All @@ -82,13 +85,10 @@ void BdxTransfer::HandleTransferSessionOutput(TransferSession::OutputEvent & eve
switch (event.EventType)
{
case TransferSession::OutputEventType::kInitReceived:
mInitData = event.transferInitData;
mDelegate->InitMessageReceived(this, mInitData);
mDelegate->InitMessageReceived(this, event.transferInitData);
break;
case TransferSession::OutputEventType::kStatusReceived:
ChipLogError(BDX, "Received StatusReport %x", static_cast<uint16_t>(event.statusData.statusCode));
// TODO: Not a great error type. The issue isn't internal, it's external. We can check the status code, but I don't know
// if that would produce a better error type. Maybe CHIP_ERROR_IM_STATUS_CODE_RECEIVED?
EndSession(CHIP_ERROR_INTERNAL);
break;
case TransferSession::OutputEventType::kInternalError:
Expand Down Expand Up @@ -116,8 +116,7 @@ void BdxTransfer::HandleTransferSessionOutput(TransferSession::OutputEvent & eve
EndSession(CHIP_NO_ERROR);
break;
case TransferSession::OutputEventType::kQueryWithSkipReceived:
mDataTransferredCount += event.bytesToSkip.BytesToSkip;
mDataTransferredCount = std::min(mDataTransferredCount, mDataCount);
mDataTransferredCount = std::min<size_t>(mDataTransferredCount + event.bytesToSkip.BytesToSkip, mDataCount);
// Fallthrough intentional.
case TransferSession::OutputEventType::kQueryReceived:
SendBlock();
Expand Down
5 changes: 1 addition & 4 deletions src/controller/python/chip/bdx/bdx-transfer.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
* limitations under the License.
*/

// TODO: Includes.
#include <lib/support/Span.h>
#include <messaging/ExchangeContext.h>

Expand Down Expand Up @@ -52,8 +51,7 @@ class BdxTransfer : public Responder

// Accepts the transfer. The data provided here will be sent over the transfer. This must only be called if the transfer
// receives data from this controller.
// TODO: Should the parameter be const&?
CHIP_ERROR AcceptReceive(const ByteSpan data_to_send);
CHIP_ERROR AcceptReceive(const ByteSpan & data_to_send);

// Rejects the transfer.
CHIP_ERROR Reject();
Expand All @@ -70,7 +68,6 @@ class BdxTransfer : public Responder

Delegate * mDelegate = nullptr;
bool mAwaitingAccept = false;
TransferSession::TransferInitData mInitData;

// TODO: Request the data from a data source rather than copy the data here.
uint8_t * mData = nullptr;
Expand Down

0 comments on commit 2403120

Please sign in to comment.