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.

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

Fixes for building nRF5 platform #104

Merged
merged 6 commits into from
Mar 25, 2020

Conversation

pan-apple
Copy link
Contributor

Problem

Code for nRF5 platform is not getting built.

Summary of Changes

  1. Imported missing code
  2. Removed dependency on Profiles
  3. Fix code, and makefiles
  4. Some code is heavily dependent on Profiles and other code that we are not importing. So that is still not being built (ConfigurationManager, ThreadStackManager, SoftwareUpdateManager)
  5. Some code has been removed because of dependency on Profiles. Need to analyze that code, and bring it back with new framework.

ToDo: File issues for 4, and 5 above.

fixes #80

@pan-apple
Copy link
Contributor Author

FYI @sagar-apple

@@ -131,7 +130,7 @@ class GenericConfigurationManagerImpl

ImplClass * Impl() { return static_cast<ImplClass *>(this); }

static void HashLengthAndBase64Value(Platform::Security::SHA256 & hash, const uint8_t * val, uint16_t valLen);
// static void HashLengthAndBase64Value(Platform::Security::SHA256 & hash, const uint8_t * val, uint16_t valLen);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we delete, and not comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

template<class ImplClass>
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetQRCodeString(char * buf, size_t bufSize)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
/* Commented out for time being, until we have Device Description service
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we delete then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

public:

// ===== Members that define the public interface of the ConfigurationManager

enum
{
kMaxPairingCodeLength = 15,
kMaxSerialNumberLength = ChipDeviceDescriptor::kMaxSerialNumberLength,
kMaxFirmwareRevisionLength = ChipDeviceDescriptor::kMaxSoftwareVersionLength,
kMaxSerialNumberLength = 32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Tag these with an XXX or some such for follow-up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

kMaxSerialNumberLength = ChipDeviceDescriptor::kMaxSerialNumberLength,
kMaxFirmwareRevisionLength = ChipDeviceDescriptor::kMaxSoftwareVersionLength,
kMaxSerialNumberLength = 32,
kMaxFirmwareRevisionLength = 32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Tag these with an XXX or some such for follow-up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gerickson I am going to submit GitHub issues to track these.

@@ -403,7 +403,7 @@
* using the chip Time Sync protocol.
*/
#ifndef CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC
#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 1
#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

@@ -429,16 +417,6 @@ inline CHIP_ERROR ConfigurationManager::WritePersistedStorageValue(::chip::Platf
return static_cast<ImplClass*>(this)->_WritePersistedStorageValue(key, value);
}

inline CHIP_ERROR ConfigurationManager::GetDeviceDescriptor(chipDeviceDescriptor & deviceDesc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

@@ -704,96 +701,11 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_SetFailSafeArmed(bool va
return Impl()->WriteConfigValue(ImplClass::kConfigKey_FailSafeArmed, val);
}

template<class ImplClass>
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetDeviceDescriptor(::chip::Profiles::DeviceDescription::ChipDeviceDescriptor & deviceDesc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

template<class ImplClass>
CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_GetQRCodeString(char * buf, size_t bufSize)
{
return CHIP_ERROR_NOT_IMPLEMENTED;
/* Commented out for time being, until we have Device Description service
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

@@ -969,7 +882,7 @@ CHIP_ERROR GenericConfigurationManagerImpl<ImplClass>::_ComputeProvisioningHash(
SuccessOrExit(err);

// Hash the length and value of the device certificate in base-64 form.
HashLengthAndBase64Value(hash, dataBuf, (uint16_t)certLen);
// HashLengthAndBase64Value(hash, dataBuf, (uint16_t)certLen);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

mExchangeCtx->OnKeyError = OnKeyError;

// Send the query
err = mExchangeCtx->SendMessage(kChipProfile_SWU,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

void GenericSoftwareUpdateManagerImpl<ImplClass>::HandleResponse(ExchangeContext * ec, const IPPacketInfo * pktInfo, const ChipMessageInfo * msgInfo,
uint32_t profileId, uint8_t msgType, PacketBuffer * payload)
{
GenericSoftwareUpdateManagerImpl<ImplClass> * self = &SoftwareUpdateMgrImpl();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

void GenericSoftwareUpdateManagerImpl<ImplClass>::HandleServiceBindingEvent(void * appState, ::chip::Binding::EventType eventType,
const ::chip::Binding::InEventParam & aInParam, ::chip::Binding::OutEventParam & aOutParam)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

@@ -40,7 +40,7 @@

#define CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE 1

#define CHIP_DEVICE_CONFIG_ENABLE_CHIP_TIME_SERVICE_TIME_SYNC 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

public:

// ===== Members that define the public interface of the ConfigurationManager

enum
{
kMaxPairingCodeLength = 15,
kMaxSerialNumberLength = ChipDeviceDescriptor::kMaxSerialNumberLength,
kMaxFirmwareRevisionLength = ChipDeviceDescriptor::kMaxSoftwareVersionLength,
kMaxSerialNumberLength = 32,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToDo: File issue to track this

Copy link
Contributor

@gerickson gerickson left a comment

Choose a reason for hiding this comment

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

There's a sufficiently large amount of commented out / changed code in here that I'd recommend determining a "breadcrumb" convention such that you / we know that we need to come back and revisit those breadcrumbs for those comment outs / changes.

@pan-apple
Copy link
Contributor Author

There's a sufficiently large amount of commented out / changed code in here that I'd recommend determining a "breadcrumb" convention such that you / we know that we need to come back and revisit those breadcrumbs for those comment outs / changes.

@gerickson I was thinking of filing Github issues. But I am open to following breadcrumbs. LMK whatever works best overall. Github issues have benefit that it can be tracked, without grepp'ing through the code.

@woody-apple
Copy link
Contributor

@hawk248 @BroderickCarlin ?

Copy link

@hawk248 hawk248 left a comment

Choose a reason for hiding this comment

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

👍

@woody-apple woody-apple merged commit 502fb51 into project-chip:master Mar 25, 2020
@pan-apple pan-apple deleted the nrf5-platform branch March 25, 2020 21:44
fkjagodzinski pushed a commit to fkjagodzinski/connectedhomeip that referenced this pull request Apr 8, 2021
shgutte pushed a commit to shgutte/connectedhomeip that referenced this pull request Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nRF5 platform layer code is not being built
4 participants