Skip to content

Software diagnostic should not return not enabled attributes#630

Closed
ArekBalysNordic wants to merge 12 commits into
nrfconnect:masterfrom
ArekBalysNordic:software-diagnostic-fix
Closed

Software diagnostic should not return not enabled attributes#630
ArekBalysNordic wants to merge 12 commits into
nrfconnect:masterfrom
ArekBalysNordic:software-diagnostic-fix

Conversation

@ArekBalysNordic
Copy link
Copy Markdown
Contributor

Summary

Software diagnostic should not return not enabled attributes. If the attribute is not enabled it should not be sent to the
controller. Instead it should send UnsupportedAttribute.

Testing

Tested in NCS + new unit tests

@ArekBalysNordic ArekBalysNordic requested a review from a team as a code owner July 24, 2025 12:19
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 24, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
0 out of 7 committers have signed the CLA.

❌ Alami-Amine
❌ andy31415
❌ pankore
❌ gcobs0834
❌ bzbarsky-apple
❌ soares-sergio
❌ Sarthak-Shaha
You have signed the CLA already but the status is still pending? Let us recheck it.

andy31415 and others added 12 commits July 25, 2025 07:08
… ember detailed type information (#39693)

* Start moving and renaming...no implementation updates though, so things will fail

* Start adding persistence

* Blind implementation updates

* Build GN updates

* Fix more includes

* Add more tests

* Restyle

* Documentation update

* Update assertion

* Remove constexpr. unfortunately nlio is not constexpr ... sadness

* Restyled by prettier-markdown

* Fix signature

* Fix cmake datamodep dependencies

* Fix includes

* Also update darwin path: persistence moved from util

* Fix double return

* Make the compiler happy

* Expose attribute persistence to clusters as context

* Separate out AttributePersistenceProvider Instance and expose the persistence to cluster contexts

* Restyle

* Add missing file

* Logic update: even simpler interface, leave it up to ember to validate the underlying data

* Documentation update on why we did the update

* No example really for passed in data...

* Restyle

* Restyled by clang-format

* Fix path

* updated code to fix CI and upgrading docs

* Remove odd extra file

* Remove extra odd file

* Fix qpg: add deferred include

* Update docs/upgrading.md

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

* Fix typo

* Single template class for pascal strings, with specialisations

* Make things compile

* Fix another usage of prefix length

* Better length variable types in pascal strings

* Restyled by clang-format

* Fix qpg

* Hopefully fix compile cast error

* Fix one more static cast

* make tidy happy

* Better logic

* Minor typo fix

* Minor comment updates: spell check and casing

* Update src/app/persistence/PascalString.h

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

* Try to make clang-tidy on darwin happy

* Skip tidy on TestPascalString - it segfaults...

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
…ks to avoid leaking non-existence (#39415)

* CommandHandler: returning AccessFailures before Existence Failures

* WriteHandler: returning ACL check failures before Attribute existence failures

* Adapting TC_ACE_2.2 for WriteHandler Changes

* Adapting changes to new spec

* removing TODO comment

* Adding Unit Tests

* minor fixes and gemini comments

* Unit Tests fix

* Integrating Comments
* [Realtek] Bump ot-realtek to latest

* [Realtek] Restyle files

* [Realtek] fix button event bug

* [Realtek] Optimize code

* [Realtek] Add const

* [Realtek] Add software-string-version in DeviceInfo

* [Realtek] fix bugs

* [Realtek] Optimize code
* add: chef app for  microwave oven

* change file name

* remove old files

* add: add required attribute

* fix typo

* Restyled by whitespace

* merge master

* add microwave oven control cluster impl

* Restyled by whitespace

* Restyled by clang-format

* remove multiple type inheritance

* Restyled by clang-format

* ModeTagStructType

* update pointer

* Restyled by whitespace

* Restyled by clang-format

* rebuild matter file, enable WattRating attribute

* determine endpoint when device start

* Restyled by clang-format

* chore: set FeatureMap global

* chore: Address review comments

* fix ptr

* Restyled by whitespace

* Restyled by clang-format

* chore:Build matter file

* update time; set cycle

* chore: rebuild matter file

* create instance for each endpoint

* update zap file

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Sting Chang <houlunc@google.com>
Co-authored-by: Sting Chang <33673360+stingchang@users.noreply.github.com>
* update zap

* Restyled by whitespace

* Restyled by prettier-json

---------

Co-authored-by: Restyled.io <commits@restyled.io>
* Remove MATTER_ATTRIBUTE_FLAG_SINGLETON

* Fix typo

* Remove one more singleton usage

* Comment updates

* Remove one more flag usage

* More cleanup as sideffects

* Do not use singleton anymore in zcl

* Drop singletons from zap files

* zap regen

* Make darwin happy - unused attributes if there are no attributes

* Chef timeout is too tight...move to 2 hours instead of 1. It is concerning that we are this slow though...

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
…39822)

* Do some size optimization on AttributeValueEncoder.

This tries to avoid separate template instantiations of Encode methods for
various integer types and bitmap types, instead funneling all of those into the
pair of instantiations for uint64_t and int64_t.

We had some existing machinery for the unsigned integer types that this
replaces; the bitmap and signed integer bits are new and mean that callers don't
have to manually call Raw() on their bitmaps to get the space savings.

* Address review comments.
…lusters (#39977)

* Remove some templated logic

* Update for compilation

* Update src/app/clusters/software-diagnostics-server/tests/TestSoftwareDiagnosticsCluster.cpp

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

* Use composition

* Restyled by clang-format

* make code even less complex

* make code even less complex

* Restyled by clang-format

* Update src/app/clusters/software-diagnostics-server/software-diagnostics-cluster.h

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Restyled.io <commits@restyled.io>
…r for legacy (codegen/ember) clusters (#39393)

* Add CodegenServerCluster: A ServerClusterInterface wrapper for ember clusters

* Renamed to CodegenServerCluster to ServerClusterShim, moved to examples and added a readme

* Fix mispelling and missing dependency

* Restyled by prettier-markdown

* Fix typo

* Add EventInfo

* Restyled by clang-format

* Fix some include paths

* Add prefix to include path

* Add comment about GetClusterFlags not being supported by ember

* Restyled by clang-format

* Remove unit tests from efr32 to fix CI failure on ember mocks.

* Update model.gni

* Adds app/required_privillege dependency back

This reverts commit abda30a6cdfa9b9a7f61f4e8eeec350ac2d9e3ad.

* attempt to fix missing dependency

* Remove comment about Zigbee stuff

* Address review suggestions: return UnsupportedEndpoint; Check handler for nullptr

* Restyled by clang-format

* Fix comment about command handler interface

* Fix another comment about command handler interface

* Replaced UnsupportedEndpoint with Failure; Improved logs; Added comment about DataVersion

* Restyled by whitespace

* Restyled by clang-format

* Attempt to fix CI issue related to required_privillege dep

* Attempt2 to fix required_privillege dep CI issue

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Andrei Litvin <andreilitvin@google.com>
… of DataModel::Provider (#39686)

* Ensure SeverClusterInterface validation is happening

* Add more tests

* Fix read test

* Fix copy and paste name error

* Fix model cleanup for registration

* A bit of code sharing

* remove unused function

* Fix includes

* move validity for write into the API client

* Restyle

* Fix signature

* Write interaction test fixes

* Also fix testwrite

* Remove EmberMetadata.h/cpp and update ReadAttribute requirements. Still need to fix things up

* Use VerifyOrReturnError instead of die, just in case to not crash devices

* Logic update and restyle

* Fix typo

* Comment update

* update metadata some more

* Conditional infinite reads. KillOverQuota passes... still need to check the parallel reads

* Unit tests now pass

* Restyled by clang-format

* Fix include

* Fix comments

* Fix comments

* Update comments to require ACL checks

* Fix return value for reads on write-only values

* Comment and naming update

* Adding explicit reference

* Remove unused function

* Comment update and remove unnecessary check

* Better spec alignment

* make things compile

* Fix compile

* make comments consistent

* Update headers

* Undo submodule change

* Fix include

* Fix merge differences in metadata handling

* one more compile fix

* Update src/app/WriteHandler.cpp

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

* Update src/app/data-model-provider/Provider.h

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

* Remove extra todo

* Restyle

* Update src/app/data-model-provider/Provider.h

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

* Update src/app/data-model-provider/Provider.h

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

* One more compile error

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
…use for server cluster implementations and reduce flash overhead (#40202)

* Fix very common case of adding mandatory and optional attributes

* Fix bugs

* Restyle

* Fix typo

* Remove obsolete code

* Some logic cleanup

* Remove extra method

* Restyle

* Update src/app/server-cluster/DefaultServerCluster.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/app/server-cluster/DefaultServerCluster.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Restyle

* use reference

* Fix logic error

* Update src/app/clusters/administrator-commissioning-server/AdministratorCommissioningCluster.cpp

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

* Update src/app/server-cluster/DefaultServerCluster.h

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

* Add comments

* Update src/app/server-cluster/DefaultServerCluster.cpp

Co-authored-by: Sergio Soares <sergiosoares@google.com>

* Refactor code into a separate AttributeListBuilder

* Enable unit tests

* Update src/app/server-cluster/AttributeListBuilder.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Restyle

* fix cmake

---------

Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Sergio Soares <sergiosoares@google.com>
@ArekBalysNordic ArekBalysNordic force-pushed the software-diagnostic-fix branch from 4caf9b4 to d37a175 Compare July 25, 2025 09:30
ArekBalysNordic added a commit to ArekBalysNordic/sdk-nrf that referenced this pull request Jul 25, 2025
From PR: nrfconnect/sdk-connectedhomeip#630

Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>
@LuDuda
Copy link
Copy Markdown
Collaborator

LuDuda commented Aug 1, 2025

Huh.. 12k+ lines of changes.. Do we need it for NCS 3.1.0 and certification, or it could go afterwards?

@doublemis1
Copy link
Copy Markdown
Contributor

I think we decided to drop these cherry-picks for NCS 3.1.0 and just enable all attributes and features for software diagnostics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.