Rename DataModel to InteractionModel, introduce InteractionModelState uniting Events, Subscriptions, Networks and others.#485
Open
ivmarkov wants to merge 14 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request consolidates the data model's state—including the subscriptions table, events queue, and network store—into a single unified 'DataModelState' struct (along with its specialized type aliases 'EthDataModelState' and 'WirelessDataModelState'). This change simplifies the 'DataModel::new' constructor by replacing multiple separate arguments with a single reference to the consolidated state. Consequently, numerous example binaries, tests, and internal references have been refactored to instantiate and use this unified state struct, reducing boilerplate and improving code organization. I have no feedback to provide as there are no review comments.
|
PR #485: Size comparison from 256033b to 3b6b540 Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
|
PR #485: Size comparison from 256033b to 3b5a27d Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
ivmarkov
force-pushed
the
data-model-state
branch
from
f16bdfe to
230a022
Compare
|
PR #485: Size comparison from 256033b to 230a022 Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
ivmarkov
force-pushed
the
data-model-state
branch
2 times, most recently
from
c4d9ff8 to
34d1d65
Compare
|
PR #485: Size comparison from 13a6675 to 34d1d65 Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
ivmarkov
force-pushed
the
data-model-state
branch
from
b056b7a to
8096b2b
Compare
ivmarkov
changed the title
|
PR #485: Size comparison from f3044ab to 8096b2b Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
ivmarkov
force-pushed
the
data-model-state
branch
from
1df8d1e to
cfc14c5
Compare
|
PR #485: Size comparison from db10663 to 893c9ea Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
|
PR #485: Size comparison from db10663 to cbad7d1 Increases above 0.2%:
Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #411
Goal is to simplify the life of the user by not asking him to think about too many structs related to Matter internals.
=========
EDIT: As part of this relatively large change, I thought I'd bite the bullet and do the following renames / file moves as part of this PR or else all clients will have to do two migrations - one, to the
DataModelState, and then another shortly after - to the newInteraction*names:dm::DataModel->im::InteractionModeldm::DataModelState->im::InteractionModelStatedm::DataModelHandler->dm::DataModel(now that theDataModelname is available)And also (but these should not concern users, as subscriptions and events are now part of the State struct):
dm::Subscriptions->im::Subscriptionsdm::Events->im::EventsFurther, what used to be knows as the
immodule is now inim::encoding(except theBusyInteractionModelwhich is now lined-up as a sibling ofInteractionModel).That's because the
immodule really only used to contain the "encoding" IM types, like AttrPath, CmdPath, NodeId, EndptId and so on. Moving these toim::encodingallowed us to establish the following dependency chain:im->dm-> (a tiny subset of)im::encodingwhere:
im=InteractionModel, InteractionModelState,BusyInteractionModel` etc.dm=DataModel,(Async)Handler,Metadataand many othersim::encoding= NodeId, EndptId, ClusterIdm AttrId, CmdIdm, AttrPath, CmdPath and similarThe justification for these type renames and migrations to the
immodule is that they match more closely what these types actually do. What (used to be) calledDataModelis in reality theInteractionModelExchange handler. I.e. the thing which responds to IM messages coming via exchanges.The combo of
(Async)Handler+Metadatais really our data model (per Matter spec).Though the
*Handlertype still contains the "handling" verbs' read/write/invoke/subscribe, these are now at an individual command/attribute level, rather than dealing with the IM protocol message payloads directly.Now, this rename introduces circular dependencies between the
dmandimmodules, in that theInteractionModelwould obviously reference and use the(Async)Handler+Metadataand other supporting traits defined in thedmmodule, but also the other way around, where some basic types defined in theimmodule would be used by thedmmodule, like type aliases for node/endpoint/cluster/attr/cmd IDs, as well as some IM structs, likeAttrPath,CmdPathand others. But I don't see a way around that. It is not worth it to completely abstract the data model from these IM types.