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

Window app rework #1

Closed
wants to merge 126 commits into from

Conversation

andygallay-silabs
Copy link
Owner

No description provided.

andy31415 and others added 20 commits May 29, 2023 15:24
* Add JAVA_PATH to our dockerfile environment variables

* Use matter_enable_java_compilation instead of build_java_matter_controller

* Remove useless bit

* Fix unit tests once we have java available

* Fix seting up java deps if files already there ... this now runs in 5ms instead of 400ms

* Do not re-download java dependencies every time.

This moves java dependency download time from 500ms to 5ms after
the initial download.

If we get more dependencies, the savings should be even better.

* Restyled by shellharden

* Restyled by shfmt

* Prepare for unit tests

* Make output class directory unique for java

* Restyled by gn

* Undo unintentional change

* Allow separate unit test runs for java

* Restyled by gn

* Restyled by shellharden

* Restyled by shfmt

* Undo unintentional change in test name

* Fix yaml file

* Force java path

* Attempt to fix the run path ... again

* Update to not use run_in_build_env and switch to multi-line run script

* Do not attempt to compile pure java variant for android (keep only linux and mac generally)

* Fix required jar: tlv now needs copy as the tlv content is not part of controller anymore

* Ensure unit tests pretend java path is set

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>
* Align Thread Network Diagnostics XML to the spec.

Spec changes happened in
CHIP-Specifications/connectedhomeip-spec#6169 and
CHIP-Specifications/connectedhomeip-spec#6338

Fixes project-chip#25339

* Auto-update ZAP files.

* Regenerate generated code.
* Updated fan-control-server with better support for feature checking, step command handler and checks around the supported attributes in the pre-callback

* Updated fan-control-cluster.xml for TE1 for fall release and regenerated the generated code

* Ensured status was always initialised before returning, added missing break in switch case

* Update optional parameters of Step command and matched spec naming. Improved checking on Auto feature when receiving smart mode.

* Added a stop-gap fan-stub.cpp so that the existing FanControl tests pass for TE1

* removed unused variable

* fixed return after else warning

* reordered files as per restyler
This uses std::from_chars which is a C++17 feature.
Since this is linux, it is safe to say C++17 or higher.

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
* Skip restyling java generated files

* Run zap_regen_all

* Fix restyle path to include a glob

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
* add unbolt support to door lock cluster

* Adjust Darwin availability annotation to fix codegen failure.

* add unbolt callback to door lock server

* handle setting the LockState to Unlatched

* apply restyled.io changes

* set lock state to Unlocked after Unlatch operation

* update door lock tests to handle Unlatched state

* update door lock tests

* move handling of latch pulling to door lock example

* send lock operation event Unlatch for unlatched state

* send LockOperationEvent Unlatch immediately within HandleRemoteLockOperation

* fix duplicate test step

* apply restyled changes

* add reference to door lock server issue

---------

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
* Disabled CC26x2x7 yaml test

* Added doc to explain deactivation
* Merge for rebase onto master

* Restyled by whitespace

* fixed shadowing in InvokeCommand, modified scenes to scenes-server in CMakeList.txt

* Regenerated zap files and .matter files for lighting app with Scenes

* Modified CMakeList for lighting app to include scene src files

* Apply suggestions from code review, swapped checks on groupId != 0, replaced static_cast with to underlying, used StatusIB to convert CHIP_ERROR to status, refactored names of callbacks called from other clusters, got rid of unused functions, fixed integer promotion truncation, remove un-neccessary copies, added checks on sceneHandler failures and assertion on size for static casts.
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Removed scenes-tokens.h

* removed scenes-token.h for test's BUID.gn

* Regenerated zap files

* Rework of endpoint scope in progress

* Completed refactor of SceneTable to have per endpoint flash storage, adapted test to function with it, needs to add multi-endpoints tests

* WIP for Attributes handling, needs to add checks on status for Sets and Gets

* Refactored the scenes table and scenes server to be endpoint scoped and removed attribute interface override

* Fix multi-endpoint on scenes-server cluster, updated attribute access override, added function to add status to response and response to handler in the event of failure to shortent code

* Restyled by whitespace

* Restyled by clang-format

* Applied changes to build after rebase and regenerated zap files

* Restyled by shfmt

* Fixed uint16 to uint8 conversion issues in SceneTableImpl

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/app/clusters/groups-server/groups-server.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Removed un-necessary attribute assignation, added attribute dirtying instead. Created method to avoid code dupplication on LastConfiguredId update, addressed low hanging fruits in code style and missing checks

* Added missing description for scene storage key allocators and removed un necessary config for max transition time

* Restyled by clang-format

* Restyled by prettier-json

* Shadowing fix

* Update src/lib/support/DefaultStorageKeyAllocator.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/app/clusters/scenes-server/SceneTableImpl.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Add comment on Storage Keys, removed unused variables in comments and moved transition time add to response to after success

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Removed un-necessary check on transition time

* Restyled by whitespace

* Restyled by clang-format

* Added missing uint16_t in loop to mMaxScenesPerFabric

* Added check on nullptr in TestSceneTable

* Moved init to MatterScenesPluginServerInitCallback to allow all-cluster-app to initialize for tests

* Update src/app/clusters/scenes-server/scenes-server.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Update src/app/clusters/scenes-server/scenes-server.cpp

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* Added using for ScenesServer in Matter init callback

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Problem:
  - New PAAs were added, both in Main Net and Test Net, since last mirror
  - These were missing from the SDK

Description of change:

Ran the following from SDK root
```
  pip install click_option_group # somehow missing from
  cd credentials/development
  python ../fetch-paa-certs-from-dcl.py --use-test-net-http
  cd ../production
  python ../fetch-paa-certs-from-dcl.py --use-main-net-http
```

Testing done:
  - Validate that expected new PAAs from web UI are present
…work. (project-chip#26890)

In particular, in MTRDevice we should cache that we had a decode failure instead
of leaving the cache un-filled.
…project-chip#26928)

* Fix identify server callbacks in silabs app. Move app code to the common BaseApplication. (lighting-app done)

* Remove identify callbacks implementation from the remaining silabs appTask.cpp

* remove invalid cast on qpg platform
Use correct type to refer to key id.
Fix AES CCM encrypt/decrypt function - null arguments
validation.
Remove mbedtls PSA crypto condition - it isn't valid for
external PSA crypto source such as TF-M.

Signed-off-by: Vincent Coubard <vincent.coubard@arm.com>
…hip#26944)

* added the the PM1, PM2.5, PM10, CO2, NO2, TVOC, CH2O, CO, Ozone & Radon concentration measurement clusters to the all-clusters-app.

* Added plugin server init callback functions to util.cpp

* Added zap generated files.

* Restyled by prettier-json

* Sorted the zap_cluster_list.json

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Copy link
Collaborator

@jepenven-silabs jepenven-silabs left a comment

Choose a reason for hiding this comment

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

Rework needed. Seems like the initial implementation was simply wrapped inside an Apptask without any cleanup whatsoever. Please cleanup the Window Manager and make it more streamlined with other examples

examples/window-app/common/src/WindowApp.cpp Outdated Show resolved Hide resolved
@@ -24,4 +24,4 @@ chip_enable_openthread = true
openthread_external_platform =
"${chip_root}/third_party/openthread/platforms/efr32:libopenthread-efr32"

use_base_app = false
use_base_app = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

This build option should be removed at large since it's not needed anymore

Copy link
Owner Author

@andygallay-silabs andygallay-silabs May 31, 2023

Choose a reason for hiding this comment

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

Removed. But so far, I need to specify use_base_app = true when building the application through the terminal:

./scripts/examples/gn_efr32_example.sh ./examples/window-app/silabs/efr32/ ./out/window-app BRD4187C use_base_app=true

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be removed at large, meaning removed everywhere, even beyond the window-app...

@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import("//build_overrides/chip.gni")
import("${chip_root}/config/standalone/args.gni")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

A tentative to integrate the common window app data model to the silabs folder.

Timer(const char * name, uint32_t timeoutInMs, Callback callback, void * context);

void Start();
void Stop();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why would we want to stop the Window manager? If unused please remove

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's the Timer stop function, used in line 452 and 516, 955, 989 in WindowManager.cpp.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then couldn't we just use the FreeRTOS api directly instead of going through a wrapper ?

public:

static WindowManager sInstance;
struct Timer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this whole struct needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's the Timer struct from WindowApp (base class) and WindowAppImpl combined, since WindowAppImpl extended the definition of the base class Timer struct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That doesn't answer the question. Is is really needed ? couldn't we just use the FreeRTOS api directly ? if so please removed.

static void TimerCallback(TimerHandle_t xTimer);
};

struct Button
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this struct really needed?

Copy link
Owner Author

Choose a reason for hiding this comment

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

The struct is used within the WindowManager to define the actions of the buttons on the board. Maybe the buttons behavior could be called in AppTask, like the lightning app example ?


CHIP_ERROR Init();
CHIP_ERROR Start();
CHIP_ERROR Run();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should cleanup all of this with the Start, Run, Stop and finish

Copy link
Owner Author

@andygallay-silabs andygallay-silabs May 31, 2023

Choose a reason for hiding this comment

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

Will merge Init and Start into a single Init function. Removed Finish function since we don't care about memory leaks in our use case. I can merge the code within WindowManager::Run() into the WindowManager::OnTaskCallback where the function Run() is called.

"packages": [
{
"path": "gn/gn/${platform}",
"platforms": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file should not be committed

bzbarsky-apple and others added 7 commits May 31, 2023 13:50
These were being used by the old scenes implementation, which is now gone.
project-chip#26958)

This is ending up not working quite right because when structs have struct-typed
(or list-of-struct-typed) fields, the type expectations people have get messed
up.  They're still messed up even after this PR, but at least we're not sort of
promising they won't be messed up.

This reverts project-chip#26763 and project-chip#26495.
@@ -24,4 +24,4 @@ chip_enable_openthread = true
openthread_external_platform =
"${chip_root}/third_party/openthread/platforms/efr32:libopenthread-efr32"

use_base_app = false
use_base_app = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be removed at large, meaning removed everywhere, even beyond the window-app...


union
{
struct
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are all these struct needed

Copy link
Owner Author

Choose a reason for hiding this comment

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

Removed LightEvent

*********************************************************/

// Application-defined error codes in the CHIP_ERROR space.
#define APP_ERROR_EVENT_QUEUE_FAILED CHIP_APPLICATION_ERROR(0x01)
Copy link
Collaborator

Choose a reason for hiding this comment

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

These defines are probably not needed please removed

*
* @param identify identify structure the command applies on
*/
static void OnIdentifyStart(Identify * identify);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this will have to go after the rebase

public:

static WindowManager sInstance;
struct Timer
Copy link
Collaborator

Choose a reason for hiding this comment

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

That doesn't answer the question. Is is really needed ? couldn't we just use the FreeRTOS api directly ? if so please removed.

Timer(const char * name, uint32_t timeoutInMs, Callback callback, void * context);

void Start();
void Stop();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Then couldn't we just use the FreeRTOS api directly instead of going through a wrapper ?


Timer * CreateTimer(const char * name, uint32_t timeoutInMs, WindowManager::Timer::Callback callback,
void * context);
Button * CreateButton(WindowManager::Button::Id id, const char * name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really needed ?


#include <platform/CHIPDeviceLayer.h>

#if defined(SL_CATALOG_SIMPLE_LED_LED1_PRESENT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be removed for 917 compatibility

}
} // namespace

void OnTriggerIdentifyEffect(Identify * identify)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will have to be removed after the rebase

@andygallay-silabs andygallay-silabs force-pushed the window-app-rework branch 2 times, most recently from 82d9d03 to 20b4176 Compare June 15, 2023 18:00
andygallay-silabs added a commit that referenced this pull request Jun 19, 2023
Merged WindowApp and WindowAppImpl into WindowManager.

Buildable WindowApp example from AppTask. Needs debug.

Update AppTask.cpp

Working refactor for Window App example with common AppTask

Clean up #1 for AppTask

Clean up #2 for AppTask

Refactor Window-app
@andygallay-silabs andygallay-silabs force-pushed the window-app-rework branch 2 times, most recently from e02f99c to 4445bb5 Compare June 19, 2023 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment