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.

Sign up for GitHub

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

Add support for commisionable advertisement #4337

Merged
merged 7 commits into from
Jan 14, 2021
Merged

Add support for commisionable advertisement #4337

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
31bbe63
Add support for commisionable advertisement
arunbharadwaj Jan 12, 2021
cf3d42e
Minor fixes based on review comments
arunbharadwaj Jan 13, 2021
0e36bc2
Use a flag to enable/disable commisionable mode
arunbharadwaj Jan 13, 2021
7fd2c76
Merge branch 'master' into 02_commisionable_discovery
arunbharadwaj Jan 13, 2021
377ea28
Restyled by clang-format
restyled-commits Jan 13, 2021
c90a1c2
Revert enable/disable flag for commisionable mode
arunbharadwaj Jan 14, 2021
3c120cc
Restyled by clang-format
restyled-commits Jan 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions examples/minimal-mdns/advertiser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,21 @@ enum class AdvertisingMode
{
kCommisioning,
kOperational,
kCommisionable,
};

struct Options
{
bool enableIpV4 = false;
AdvertisingMode advertisingMode = AdvertisingMode::kCommisioning;

// commisioning params
// commisioning/commisionable params
uint8_t shortDiscriminator = 52;
uint16_t longDiscriminator = 840;
Optional<uint16_t> vendorId;
Optional<uint16_t> productId;
Optional<const char *> pairingInstr;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required for commisionable as well? if not, I think we should add a comment that these 2 are for commisionable only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only required for commisionable. Will add a comment.

Optional<uint8_t> pairingHint;

// operational params
uint64_t fabricId = 12345;
Expand All @@ -61,6 +64,8 @@ constexpr uint16_t kOptionCommisioningShordDiscriminator = 's';
constexpr uint16_t kOptionCommisioningLongDiscriminaotr = 'l';
constexpr uint16_t kOptionCommisioningVendorId = 0x100; // v is used by 'version'
constexpr uint16_t kOptionCommisioningProductId = 'p';
constexpr uint16_t kOptionCommisioningPairingInstr = 'I';
constexpr uint16_t kOptionCommisioningPairingHint = 'H';
Copy link
Contributor

Choose a reason for hiding this comment

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

using uppercase seems a bit different than existing flags. I imagine 'H' is to not conflict with 'h'. I would either then update vendorId to use 'V' (and maybe make all others uppercase too) or make the conflicting ones only have a long version like '--pairing-hint'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think sticking to the long version for conflicting ones is more readable. Will do that.


constexpr uint16_t kOptionOperationalFabricId = 'f';
constexpr uint16_t kOptionOperationalNodeId = 'n';
Expand All @@ -81,6 +86,10 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
{
gOptions.advertisingMode = AdvertisingMode::kCommisioning;
}
else if ((strcmp(aValue, "commisionable") == 0) || (strcmp(aValue, "d") == 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

the 'd' is awkward. Please remove 'c' from above instead since 'c' seems ambigous for commisiong/commisionable

Copy link
Contributor Author

@arunbharadwaj arunbharadwaj Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I used 'd' because for commisionable we advertise as _chipd while for commisioning we advertise as _chipc and hence 'c' was used.

{
gOptions.advertisingMode = AdvertisingMode::kCommisionable;
}
else
{
PrintArgError("%s: Invalid advertising mode %s\n", aProgram, aValue);
Expand All @@ -100,6 +109,12 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
case kOptionCommisioningProductId:
gOptions.productId = Optional<uint16_t>::Value(static_cast<uint16_t>(atoi(aValue)));
return true;
case kOptionCommisioningPairingInstr:
gOptions.pairingInstr = Optional<const char *>::Value(static_cast<const char *>(aValue));
return true;
case kOptionCommisioningPairingHint:
gOptions.pairingHint = Optional<uint8_t>::Value(static_cast<uint8_t>(atoi(aValue)));
return true;
case kOptionOperationalFabricId:
gOptions.fabricId = atoll(aValue);
return true;
Expand All @@ -122,6 +137,8 @@ OptionDef cmdLineOptionsDef[] = {
{ "long-discriminator", kArgumentRequired, kOptionCommisioningLongDiscriminaotr },
{ "vendor-id", kArgumentRequired, kOptionCommisioningVendorId },
{ "product-id", kArgumentRequired, kOptionCommisioningProductId },
{ "pairing-instruction", kArgumentRequired, kOptionCommisioningPairingInstr },
{ "pairing-hint", kArgumentRequired, kOptionCommisioningPairingHint },

{ "fabrid-id", kArgumentRequired, kOptionOperationalFabricId },
{ "node-id", kArgumentRequired, kOptionOperationalNodeId },
Expand All @@ -136,18 +153,24 @@ OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS"
#endif
" -m <mode>\n"
" --advertising-mode <mode>\n"
" Advertise in this mode (o/operational or c/commisioning).\n"
" Advertise in this mode (o/operational or c/commisioning or d/commisionable).\n"
" --short-discriminator <value>\n"
" -s <value>\n"
" Commisioning short discriminator.\n"
" Commisioning/commisionable short discriminator.\n"
" --long-discriminator <value>\n"
" -l <value>\n"
" Commisioning long discriminator.\n"
" Commisioning/commisionable long discriminator.\n"
" --vendor-id <value>\n"
" Commisioning vendor id.\n"
" Commisioning/commisionable vendor id.\n"
" --product-id <value>\n"
" -p <value>\n"
" Commisioning product id.\n"
" Commisioning/commisionable product id.\n"
" --pairing-instruction <value>\n"
" -I <value>\n"
" Commisionable pairing instruction.\n"
" --pairing-hint <value>\n"
" -H <value>\n"
" Commisionable pairing hint.\n"
" --fabrid-id <value>\n"
" -f <value>\n"
" Operational fabric id.\n"
Expand Down Expand Up @@ -207,6 +230,18 @@ int main(int argc, char ** args)
.SetFabricId(gOptions.fabricId)
.SetNodeId(gOptions.nodeId));
}
else if (gOptions.advertisingMode == AdvertisingMode::kCommisionable)
{
err = chip::Mdns::ServiceAdvertiser::Instance().Advertise(chip::Mdns::CommisionableAdvertisingParameters()
.EnableIpV4(gOptions.enableIpV4)
.SetPort(CHIP_PORT)
.SetShortDiscriminator(gOptions.shortDiscriminator)
.SetLongDiscrimininator(gOptions.longDiscriminator)
.SetVendorId(gOptions.vendorId)
.SetProductId(gOptions.productId)
.SetPairingInstr(gOptions.pairingInstr)
.SetPairingHint(gOptions.pairingHint));
}
else
{
fprintf(stderr, "FAILED to determine advertising type.\n");
Expand Down
57 changes: 57 additions & 0 deletions src/lib/mdns/Advertiser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,60 @@ class CommisioningAdvertisingParameters : public BaseAdvertisingParams<Commision
chip::Optional<uint16_t> mProductId;
};

class CommisionableAdvertisingParameters : public BaseAdvertisingParams<CommisionableAdvertisingParameters>
{
public:
CommisionableAdvertisingParameters & SetShortDiscriminator(uint8_t discriminator)
{
mShortDiscriminator = discriminator;
return *this;
}
uint8_t GetShortDiscriminator() const { return mShortDiscriminator; }

CommisionableAdvertisingParameters & SetLongDiscrimininator(uint16_t discriminator)
{
mLongDiscriminator = discriminator;
return *this;
}
uint16_t GetLongDiscriminator() const { return mLongDiscriminator; }

CommisionableAdvertisingParameters & SetVendorId(Optional<uint16_t> vendorId)
{
mVendorId = vendorId;
return *this;
}
Optional<uint16_t> GetVendorId() const { return mVendorId; }

CommisionableAdvertisingParameters & SetProductId(Optional<uint16_t> productId)
{
mProductId = productId;
return *this;
}
Optional<uint16_t> GetProductId() const { return mProductId; }

CommisionableAdvertisingParameters & SetPairingInstr(Optional<const char *> pairingInstr)
{
mPairingInstr = pairingInstr;
return *this;
}
Optional<const char *> GetPairingInstr() const { return mPairingInstr; }

CommisionableAdvertisingParameters & SetPairingHint(Optional<uint8_t> pairingHint)
{
mPairingHint = pairingHint;
return *this;
}
Optional<uint8_t> GetPairingHint() const { return mPairingHint; }

private:
uint8_t mShortDiscriminator = 0;
uint16_t mLongDiscriminator = 0; // 12-bit according to spec
chip::Optional<uint16_t> mVendorId;
chip::Optional<uint16_t> mProductId;
chip::Optional<const char *> mPairingInstr;
chip::Optional<uint8_t> mPairingHint;
};

/// Handles advertising of CHIP nodes
class ServiceAdvertiser
{
Expand All @@ -129,6 +183,9 @@ class ServiceAdvertiser
/// Advertises the CHIP node as a commisioning node
virtual CHIP_ERROR Advertise(const CommisioningAdvertisingParameters & params) = 0;

/// Advertises the CHIP node as a commisionable node
virtual CHIP_ERROR Advertise(const CommisionableAdvertisingParameters & params) = 0;

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a method to stop this advertisement?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not currently - we assume that 'once a chip device, always a chip device and we advertise'. Having a 'StopAdvertising' seems possible though.

/// Provides the system-wide implementation of the service advertiser
static ServiceAdvertiser & Instance();
};
Expand Down
145 changes: 145 additions & 0 deletions src/lib/mdns/Advertiser_ImplMinimalMdns.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
CHIP_ERROR Start(chip::Inet::InetLayer * inetLayer, uint16_t port) override;
CHIP_ERROR Advertise(const OperationalAdvertisingParameters & params) override;
CHIP_ERROR Advertise(const CommisioningAdvertisingParameters & params) override;
CHIP_ERROR Advertise(const CommisionableAdvertisingParameters & params) override;

// ServerDelegate
void OnQuery(const BytesRange & data, const chip::Inet::IPPacketInfo * info) override;
Expand Down Expand Up @@ -266,6 +267,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
}

FullQName GetCommisioningTextEntries(const CommisioningAdvertisingParameters & params);
FullQName GetCommisionableTextEntries(const CommisionableAdvertisingParameters & params);

static constexpr size_t kMaxEndPoints = 10;
static constexpr size_t kMaxRecords = 16;
Expand Down Expand Up @@ -559,6 +561,149 @@ FullQName AdvertiserMinMdns::GetCommisioningTextEntries(const CommisioningAdvert
return AllocateQName(txtDiscriminator, txtVidPid);
}

CHIP_ERROR AdvertiserMinMdns::Advertise(const CommisionableAdvertisingParameters & params)
{
Clear();

// TODO: need to detect colisions here
char nameBuffer[64] = "";
size_t len = snprintf(nameBuffer, sizeof(nameBuffer), "chip-%016" PRIX64, GetRandU64());
if (len >= sizeof(nameBuffer))
{
return CHIP_ERROR_NO_MEMORY;
}

FullQName operationalServiceName = AllocateQName("_chipd", "_udp", "local");
FullQName operationalServerName = AllocateQName(nameBuffer, "_chipd", "_udp", "local");
FullQName serverName = AllocateQName(nameBuffer, "local");

if ((operationalServiceName.nameCount == 0) || (operationalServerName.nameCount == 0) || (serverName.nameCount == 0))
{
ChipLogError(Discovery, "Failed to allocate QNames.");
return CHIP_ERROR_NO_MEMORY;
}

if (!AddResponder<PtrResponder>(operationalServiceName, operationalServerName)
.SetReportAdditional(operationalServerName)
.SetReportInServiceListing(true)
.IsValid())
{
ChipLogError(Discovery, "Failed to add service PTR record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}

if (!AddResponder<SrvResponder>(SrvResourceRecord(operationalServerName, serverName, params.GetPort()))
.SetReportAdditional(serverName)
.IsValid())
{
ChipLogError(Discovery, "Failed to add SRV record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}
if (!AddResponder<IPv6Responder>(serverName).IsValid())
{
ChipLogError(Discovery, "Failed to add IPv6 mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}

if (params.IsIPv4Enabled())
{
if (!AddResponder<IPv4Responder>(serverName).IsValid())
{
ChipLogError(Discovery, "Failed to add IPv4 mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}
}

{
sprintf(nameBuffer, "S%03d", params.GetShortDiscriminator());
FullQName shortServiceName = AllocateQName(nameBuffer, "_sub", "_chipd", "_udp", "local");
ReturnErrorCodeIf(shortServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY);

if (!AddResponder<PtrResponder>(shortServiceName, operationalServerName)
.SetReportAdditional(operationalServerName)
.SetReportInServiceListing(true)
.IsValid())
{
ChipLogError(Discovery, "Failed to add short discriminator PTR record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}
}

{
sprintf(nameBuffer, "L%04d", params.GetLongDiscriminator());
FullQName longServiceName = AllocateQName(nameBuffer, "_sub", "_chipd", "_udp", "local");
ReturnErrorCodeIf(longServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY);
if (!AddResponder<PtrResponder>(longServiceName, operationalServerName)
.SetReportAdditional(operationalServerName)
.SetReportInServiceListing(true)
.IsValid())
{
ChipLogError(Discovery, "Failed to add long discriminator PTR record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}
}

if (params.GetVendorId().HasValue())
{
sprintf(nameBuffer, "V%d", params.GetVendorId().Value());
FullQName vendorServiceName = AllocateQName(nameBuffer, "_sub", "_chipd", "_udp", "local");
ReturnErrorCodeIf(vendorServiceName.nameCount == 0, CHIP_ERROR_NO_MEMORY);

if (!AddResponder<PtrResponder>(vendorServiceName, operationalServerName)
.SetReportAdditional(operationalServerName)
.SetReportInServiceListing(true)
.IsValid())
{
ChipLogError(Discovery, "Failed to add vendor discriminator PTR record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}
}

if (!AddResponder<TxtResponder>(TxtResourceRecord(operationalServerName, GetCommisionableTextEntries(params)))
.SetReportAdditional(serverName)
.IsValid())
{
ChipLogError(Discovery, "Failed to add TXT record mDNS responder");
return CHIP_ERROR_NO_MEMORY;
}

ChipLogProgress(Discovery, "CHIP minimal mDNS configured as 'Commisionable device'.");

return CHIP_NO_ERROR;
}

FullQName AdvertiserMinMdns::GetCommisionableTextEntries(const CommisionableAdvertisingParameters & params)
{
// a discriminator always exists
char txtDiscriminator[32];
sprintf(txtDiscriminator, "D=%d", params.GetLongDiscriminator());

if (!params.GetVendorId().HasValue())
{
return AllocateQName(txtDiscriminator);
}

// Need to also set a vid/pid string
char txtVidPid[64];
if (params.GetProductId().HasValue())
{
sprintf(txtVidPid, "V=%d+%d", params.GetVendorId().Value(), params.GetProductId().Value());
}
else
{
sprintf(txtVidPid, "V=%d", params.GetVendorId().Value());
}

char txtPairingInstrHint[128];
if (params.GetPairingHint().HasValue())
{
// sprintf(txtPairingInstrHint, "P=%d", params.GetPairingHint().Value());
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented out code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

sprintf(txtPairingInstrHint, "P=%s+%d", params.GetPairingInstr().Value(), params.GetPairingHint().Value());
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to ensure that GetPairingInstr() HasValue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

}

return AllocateQName(txtDiscriminator, txtVidPid, txtPairingInstrHint);
}

AdvertiserMinMdns gAdvertiser;
} // namespace

Expand Down
6 changes: 6 additions & 0 deletions src/lib/mdns/Advertiser_ImplNone.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ class NoneAdvertiser : public ServiceAdvertiser
ChipLogError(Discovery, "mDNS advertising not available. Commisioning Advertisement failed.");
return CHIP_ERROR_NOT_IMPLEMENTED;
}

CHIP_ERROR Advertise(const CommisionableAdvertisingParameters & params) override
{
ChipLogError(Discovery, "mDNS advertising not available. Commisionable Advertisement failed.");
return CHIP_ERROR_NOT_IMPLEMENTED;
}
};

NoneAdvertiser gAdvertiser;
Expand Down