-
Notifications
You must be signed in to change notification settings - Fork 3
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
Develop dual fix customer #358
Conversation
haiwentelink
commented
Dec 23, 2024
3e37ea6
to
e0255ce
Compare
1ac3915
to
1f690c8
Compare
1f690c8
to
e7b9e81
Compare
0d91545
to
53353ae
Compare
f8878a1
to
8240c09
Compare
8240c09
to
2975a9d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's generally okay on code style.
Please add some content into this PR description.
Regarding the comments, you may refine them if have spare time.
@@ -265,6 +265,11 @@ config COMPRESS_LZMA | |||
default y if SOC_RISCV_TELINK_TL321X | |||
default n | |||
|
|||
config CUSTOMER_MODE | |||
bool "control the demo code" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to replace it with "Enable/Disable Customer Mode"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@damien0x0023 should not use enable , so change to control , or CI wil fail which will check enable words in descriptor.
@@ -33,6 +33,14 @@ using namespace chip::app::Clusters; | |||
void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & attributePath, uint8_t type, uint16_t size, | |||
uint8_t * value) | |||
{ | |||
#if CONFIG_CUSTOMER_MODE | |||
/* user mode , add the customer code here for cb*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
customer mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
#elif CONFIG_SOC_RISCV_TELINK_TL321X | ||
#define ZB_NVS_PARTITION slot1_partition | ||
#define ZB_NVS_PARTITION_DEVICE FIXED_PARTITION_DEVICE(ZB_NVS_PARTITION) | ||
#define ZB_NVS_START_ADR FIXED_PARTITION_OFFSET(ZB_NVS_PARTITION) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, we may move the line 49&50 after the condition clauses so that we can remove the line 54&55.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
@@ -143,6 +146,8 @@ void FactoryResetExtHandler(void) | |||
{ | |||
// Erase the user parameters partition to reset mode settings | |||
flash_erase(flash_para_dev, USER_PARTITION_OFFSET, USER_PARTITION_SIZE); | |||
// Need to erase zb nvs part in factory mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Erase ZigBee NVS data during factory reset"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
if (user_para.val == USER_ZB_SW_VAL) | ||
{ | ||
sBoot_zb = 1; | ||
/* if switch from zb , need to get all the cluster info from zb */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If switching from ZigBee mode, get all existing clusters information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
@@ -257,25 +273,28 @@ CHIP_ERROR AppTaskCommon::StartApp(void) | |||
* a given timeframe. | |||
*/ | |||
flash_read(flash_para_dev, USER_PARTITION_OFFSET, &user_para, sizeof(user_para)); | |||
/* Boot from Zigbee , need to clean the user parameters sector first and set a flag */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If booting from Zigbee mode, set a flag and clean deprecated parameter sectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
@@ -333,13 +352,16 @@ CHIP_ERROR AppTaskCommon::InitCommonParts(void) | |||
CHIP_ERROR err; | |||
|
|||
PrintFirmwareInfo(); | |||
#if CONFIG_CUSTOMER_MODE | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to add some description here to remind developers, e.g. "/* Leave empty condition clause for development on customers' boards */"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
@@ -823,6 +845,24 @@ void AppTaskCommon::ChipEventHandler(const ChipDeviceEvent * event, intptr_t /* | |||
#if CONFIG_DUAL_MODE_SWTICH | |||
case DeviceEventType::kCommissioningComplete: { | |||
uint8_t val = USER_MATTER_PAIR_VAL; | |||
#if CONFIG_CUSTOMER_MODE | |||
/* need to add here to update the cluster information , only in the zb switch and touchlink is paired*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update all cluster information, only if switching from ZigBee mode and the touchlink is paired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
if (user_para.val == USER_ZB_SW_VAL && user_para.on_net) | ||
{ | ||
Protocols::InteractionModel::Status status; | ||
/* Switch from the touch link, need to restore previous values */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If switching from the touch link, restore the previous light status values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will change
- clean the boot flag in init part . - add the kDnsstimer to protect from init to dnss . - add special proc for ikea. - add the user-para proc in matter. - add 2m flash for buteo ,and 4m for b92. - update CI for b92 and tl3218x . - fix the switch logic part . Signed-off-by: Haiwen Xia <[email protected]>
2975a9d
to
55d3a7b
Compare