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

Include identify server cluster into lighting-app #8512

Conversation

markus-becker-tridonic-com
Copy link
Contributor

@markus-becker-tridonic-com markus-becker-tridonic-com commented Jul 20, 2021

Problem

Identify server cluster is not included in any example. If included it would not compile, see e.g. #6722 which disables Identify for pump-app.

Change overview

Enable Identify server cluster in lighting-app

  • sample implementation in lighting-app/nrfconnect
  • pulls in zll-identify-server because of
    emberAfIdentifyClusterTriggerEffectCallback

TODO:

  • enable Identify cluster in all-clusters-app
  • include Identify cluster in non-nrfconnect targets
  • allow overriding emberAfPluginIdentifyStartFeedbackCallback()/emberAfPluginIdentifyStopFeedbackCallback(EndpointId endpoint), e.g. by making the functions weak,

Testing

How was this tested? (at least one bullet point required)

  • If unit tests were added, how do they cover this issue?
  • If unit tests existed, how were they fixed/modified to prevent this in future?
  • If new unit tests are not added, why not?
  • If integration tests were added, how do they verify this change?
  • If new integration tests are not added, why not?
  • If manually tested, what platforms controller and device platforms were manually tested, and how?
    Testing on-going on nrf52840dk.
  • If no testing is required, why not?

@todo
Copy link

todo bot commented Jul 20, 2021

(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.

// TODO(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.
wasHandled = emberAfIdentifyClusterIdentifyCallback(apCommandObj, identifyTime);
}
break;
}
case Clusters::Identify::Commands::Ids::IdentifyQuery: {
// TODO(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.
wasHandled = emberAfIdentifyClusterIdentifyQueryCallback(apCommandObj);
break;
}


This comment was generated by todo based on a TODO comment in 96236bb in #8512. cc @markus-becker-tridonic-com.

@todo
Copy link

todo bot commented Jul 20, 2021

(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.

// TODO(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.
wasHandled = emberAfIdentifyClusterIdentifyQueryCallback(apCommandObj);
break;
}
case Clusters::Identify::Commands::Ids::TriggerEffect: {
expectArgumentCount = 2;
uint8_t effectId;
uint8_t effectVariant;
bool argExists[2];
memset(argExists, 0, sizeof argExists);


This comment was generated by todo based on a TODO comment in 96236bb in #8512. cc @markus-becker-tridonic-com.

@todo
Copy link

todo bot commented Jul 20, 2021

(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.

// TODO(#5098) We should pass the Command Object and EndpointId to the cluster callbacks.
wasHandled = emberAfIdentifyClusterTriggerEffectCallback(apCommandObj, effectId, effectVariant);
}
break;
}
default: {
// Unrecognized command ID, error status will apply.
chip::app::CommandPathParams returnStatusParam = { aEndpointId,
0, // GroupId
Clusters::Identify::Id, aCommandId,
(chip::app::CommandPathFlags::kEndpointIdValid) };


This comment was generated by todo based on a TODO comment in 96236bb in #8512. cc @markus-becker-tridonic-com.

@stale
Copy link

stale bot commented Jul 29, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Jul 29, 2021
* sample implementation in lighting-app/nrfconnect
* pulls in zll-identify-server because of
   emberAfIdentifyClusterTriggerEffectCallback

Restyled by clang-format
@markus-becker-tridonic-com markus-becker-tridonic-com force-pushed the upstream-identify-lighting-app-master branch from 039b1b1 to 8eda646 Compare August 2, 2021 14:42
@stale stale bot removed the stale Stale issue or PR label Aug 2, 2021
@github-actions
Copy link

github-actions bot commented Aug 2, 2021

Size increase report for "esp32-example-build" from 4f6ac3a

File Section File VM
chip-temperature-measurement-app.elf .flash.text -60 -60
chip-ipv6only-app.elf .flash.text 172 172
chip-shell.elf .flash.text 24 24
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_loc,0,197
.debug_info,0,139
.debug_line,0,138
.debug_str,0,102
.debug_frame,0,44
.debug_aranges,0,8
.debug_ranges,0,8

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,157
.debug_line,0,108
.debug_str,0,100
[Unmapped],0,60
.debug_loc,0,59
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.flash.text,-60,-60

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize
.flash.text,172,172
[Unmapped],0,-172

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.flash.text,24,24
[Unmapped],0,-24

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_info,0,157
.debug_line,0,108
.debug_str,0,104
.debug_loc,0,59
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize


@stale
Copy link

stale bot commented Aug 9, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Aug 9, 2021
@stale
Copy link

stale bot commented Aug 16, 2021

This stale pull request has been automatically closed. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app examples stale Stale issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants