[controller] Use short lived CommandSender when no command sender is provided

[erjiaqing]

Problem

• The controller::Command has a mpCommandSender object, which is used for App provided command sender object, currently, this is used by python device controller. In however, the controller::Command is designed to be able to encode and send commands for multple used.

Change overview

• Update template
  • After this PR, the controller::Command's behavior for sending commands is
    1. Allocate a new command sender object, and throw it (make IMEngine manage its lifespan)

Testing

• The correctness of this test are already covered by various tests, like Darwin test.
• However, the issue that this PR intent to solve is not covered, and reqiures real tests since it happens in NetworkCommissioning step, and NetworkCommissioning requires available Thread network devices at least.
• It can be tested by Cirque since we can simulate full commissioning routine on Cirque.
• Added a simple test in darwin to test if we can reuse the CHIPCluster objects.

@bzbarsky-apple Can you test this on iOS apps? Thanks.

[woody-apple]

Thanks! This feels like some unit testing is required here. What unit tests can be put in place?

[bzbarsky-apple]

So this goes back to my question about lifetime of mpCommandSender on Slack. Why do we have it at all if it's single-use but the objects that are using it might be long-lived and get used more than once?

Why can't we just remove the mpCommandSender bit? Where is the restriction on lifetimes if a command sender is caller-provided documented?

What I'd like to see here is:

1. A better commit message. The PR description is closer, but doesn't really answer my questions above.
2. Better documentation at the places that provide a command sender, since that sender will still auto-destroy itself and then we will have dangling refs...

[bzbarsky-apple]

As far as testing on iOS, I don't have a setup for that right now, and I will see what I can do. But this definitely feels like something we should be able to have automated tests for.

[erjiaqing]

Thanks! This feels like some unit testing is required here. What unit tests can be put in place?

The code in this change is already covered in various tests, like tests in Darwin platform.

However, the bug it intend to solve is something happened on application side, so it is hard to be covered by UT.

Anyway, added a test in Darwin, although it is not UT, but some kind of end-to-end test.

[erjiaqing]

Thanks! This feels like some unit testing is required here. What unit tests can be put in place?

In this case,

So this goes back to my question about lifetime of mpCommandSender on Slack. Why do we have it at all if it's single-use but the objects that are using it might be long-lived and get used more than once?

Why can't we just remove the mpCommandSender bit? Where is the restriction on lifetimes if a command sender is caller-provided documented?

What I'd like to see here is:

1. A better commit message. The PR description is closer, but doesn't really answer my questions above.
2. Better documentation at the places that provide a command sender, since that sender will still auto-destroy itself and then we will have dangling refs...

Removing mpCommandSender from CHIPCluster now, now the commit message is "Not exposing command sender to app", should be clear enough now.

Also added a comment in CommandSender.

[bzbarsky-apple]

It would be a good idea to reuse a number of times larger than CHIP_MAX_NUM_COMMAND_SENDER so that we catch cases when we're leaking senders. Followup is OK for this.

[erjiaqing]

#7074 Filed.

We have tests now

[erjiaqing]

@andy31415 @Damian-Nordic PTAL, thanks.