Skip to content

Commit

Permalink
#### Problem (#5319)
Browse files Browse the repository at this point in the history
- `BitFlags.h` has relatively unsafe deprecated functions.
- Some bit flags use `enum` rather than type-safe `enum class`.

#### Summary of Changes

- Removed deprecated `SetFlags()`, `ClearFlags()`, and `GetFlags()`,
  replacing uses with `BitFlags`.

- BitFlags.h changes:
  - Default to use `std::underlying_type` rather than requiring it as a template parameter.
  - Added `BitFlags` operations to reduce uses of type-unsafe `Raw()` and `SetRaw()`:
    - Multi-argument constructors.
    - `operator FlagsEnum()`.
    - `ClearAll()`.
    - `HasAny()` and `HasAll()`, following the pattern of the existing `HasOnly()`.
    - Binary `&`.

- Converted various bit flag types from `enum` to `enum class`:
  - BLEEndPoint::ConnectionStateFlags
  - BLEEndPoint::TimerStateFlags
  - BLEManagerImpl::Flags
  - Command::CommandPathFlags
  - Encoding::HexFlags
  - GenericConfigurationManagerImpl::Flags
  - GenericConnectivityManagerImpl_Thread::Flags
  - GenericConnectivityManagerImpl_WiFi::ConnectivityFlags
  - bdx::RangeControlFlags
  - bdx::StatusCode
  - bdx::TransferControlFlags
  - bdx::TransferRole

Reland of 4b8d558 (#5232) to a logic error in `*/BLEManagerImpl.cpp` and a large
accidental deletion in `K32W/BLEManagerImpl.cpp`.
  • Loading branch information
kpschoedel authored Mar 11, 2021
1 parent d8a2d1f commit 8ea423a
Show file tree
Hide file tree
Showing 59 changed files with 1,293 additions and 1,200 deletions.
18 changes: 5 additions & 13 deletions src/app/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,10 @@ chip::TLV::TLVWriter & Command::CreateCommandDataElementTLVWriter()
return mCommandDataWriter;
}

CHIP_ERROR Command::AddCommand(chip::EndpointId aEndpintId, chip::GroupId aGroupId, chip::ClusterId aClusterId,
chip::CommandId aCommandId, uint8_t aFlags)
CHIP_ERROR Command::AddCommand(chip::EndpointId aEndpointId, chip::GroupId aGroupId, chip::ClusterId aClusterId,
chip::CommandId aCommandId, BitFlags<CommandPathFlags> aFlags)
{
CommandParams commandParams;

memset(&commandParams, 0, sizeof(CommandParams));

commandParams.EndpointId = aEndpintId;
commandParams.GroupId = aGroupId;
commandParams.ClusterId = aClusterId;
commandParams.CommandId = aCommandId;
commandParams.Flags = aFlags;
CommandParams commandParams(aEndpointId, aGroupId, aClusterId, aCommandId, aFlags);

return AddCommand(commandParams);
}
Expand Down Expand Up @@ -191,12 +183,12 @@ CHIP_ERROR Command::AddCommand(CommandParams & aCommandParams)
CommandDataElement::Builder commandDataElement =
mInvokeCommandBuilder.GetCommandListBuilder().CreateCommandDataElementBuilder();
CommandPath::Builder commandPath = commandDataElement.CreateCommandPathBuilder();
if (aCommandParams.Flags & kCommandPathFlag_EndpointIdValid)
if (aCommandParams.Flags.Has(CommandPathFlags::kEndpointIdValid))
{
commandPath.EndpointId(aCommandParams.EndpointId);
}

if (aCommandParams.Flags & kCommandPathFlag_GroupIdValid)
if (aCommandParams.Flags.Has(CommandPathFlags::kGroupIdValid))
{
commandPath.GroupId(aCommandParams.GroupId);
}
Expand Down
25 changes: 16 additions & 9 deletions src/app/Command.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
* Copyright (c) 2020-2021 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -29,6 +29,7 @@
#include <messaging/ExchangeMgr.h>
#include <messaging/Flags.h>
#include <protocols/Protocols.h>
#include <support/BitFlags.h>
#include <support/CodeUtils.h>
#include <support/DLLUtil.h>
#include <support/logging/CHIPLogging.h>
Expand Down Expand Up @@ -59,25 +60,31 @@ class Command
Sending, //< The invoke command message has sent out the invoke command
};

enum class CommandPathFlags : uint8_t
{
kEndpointIdValid = 0x01, /**< Set when the EndpointId field is valid */
kGroupIdValid = 0x02, /**< Set when the GroupId field is valid */
};

/**
* Encapsulates arguments to be passed into SendCommand().
*
*/
struct CommandParams
{
CommandParams(chip::EndpointId endpointId, chip::GroupId groupId, chip::ClusterId clusterId, chip::CommandId commandId,
const BitFlags<CommandPathFlags> & flags) :
EndpointId(endpointId),
GroupId(groupId), ClusterId(clusterId), CommandId(commandId), Flags(flags)
{}

chip::EndpointId EndpointId;
chip::GroupId GroupId;
chip::ClusterId ClusterId;
chip::CommandId CommandId;
uint8_t Flags;
BitFlags<CommandPathFlags> Flags;
};

enum CommandPathFlags
{
kCommandPathFlag_EndpointIdValid = 0x0001, /**< Set when the EndpointId field is valid */
kCommandPathFlag_GroupIdValid = 0x0002, /**< Set when the GroupId field is valid */
} CommandPathFlags;

/**
* Initialize the Command object. Within the lifetime
* of this instance, this method is invoked once after object
Expand Down Expand Up @@ -110,7 +117,7 @@ class Command

chip::TLV::TLVWriter & CreateCommandDataElementTLVWriter();
CHIP_ERROR AddCommand(chip::EndpointId aEndpintId, chip::GroupId aGroupId, chip::ClusterId aClusterId,
chip::CommandId aCommandId, uint8_t Flags);
chip::CommandId aCommandId, BitFlags<CommandPathFlags> Flags);
CHIP_ERROR AddCommand(CommandParams & aCommandParams);
CHIP_ERROR AddStatusCode(const uint16_t aGeneralCode, const uint32_t aProtocolId, const uint16_t aProtocolCode,
const chip::ClusterId aClusterId);
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
* Copyright (c) 2020-2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -90,7 +90,7 @@ CHIP_ERROR SendCommandRequest(void)
kTestGroupId, // GroupId
kTestClusterId, // ClusterId
kTestCommandId, // CommandId
(chip::app::Command::kCommandPathFlag_EndpointIdValid) };
(chip::app::Command::CommandPathFlags::kEndpointIdValid) };

// Add command data here

Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/integration/chip_im_responder.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020 Project CHIP Authors
* Copyright (c) 2020-2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -60,7 +60,7 @@ void DispatchSingleClusterCommand(chip::ClusterId aClusterId, chip::CommandId aC
kTestGroupId, // GroupId
kTestClusterId, // ClusterId
kTestCommandId, // CommandId
(chip::app::Command::kCommandPathFlag_EndpointIdValid) };
(chip::app::Command::CommandPathFlags::kEndpointIdValid) };

// Add command data here

Expand Down
2 changes: 1 addition & 1 deletion src/app/util/ember-compatibility-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ bool IMEmberAfSendDefaultResponseWithCallback(EmberAfStatus status)
0, // GroupId
imCompatibilityEmberApsFrame.clusterId,
imCompatibilityEmberAfCluster.commandId,
(chip::app::Command::kCommandPathFlag_EndpointIdValid) };
(chip::app::Command::CommandPathFlags::kEndpointIdValid) };

chip::TLV::TLVType dummyType = chip::TLV::kTLVType_NotSpecified;
chip::TLV::TLVWriter writer = currentCommandObject->CreateCommandDataElementTLVWriter();
Expand Down
2 changes: 1 addition & 1 deletion src/app/zap-templates/templates/chip/CHIPClusters-src.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ CHIP_ERROR {{asCamelCased clusterName false}}Cluster::{{asCamelCased name false}
(void) onFailureCallback;

app::Command::CommandParams cmdParams = { mEndpoint, /* group id */ 0, mClusterId, k{{asCamelCased name false}}CommandId,
(chip::app::Command::kCommandPathFlag_EndpointIdValid) };
(chip::app::Command::CommandPathFlags::kEndpointIdValid) };
app::Command * ZCLcommand = mDevice->GetCommandSender();

TLV::TLVWriter writer = ZCLcommand->CreateCommandDataElementTLVWriter();
Expand Down
Loading

0 comments on commit 8ea423a

Please sign in to comment.