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

Node is too generic, use RootNode #9

Closed
jonsmirl opened this issue Jul 29, 2022 · 7 comments
Closed

Node is too generic, use RootNode #9

jonsmirl opened this issue Jul 29, 2022 · 7 comments
Labels
documentation Improvements or additions to documentation

Comments

@jonsmirl
Copy link
Contributor

These should be called RootNode to follow spec.

/* Create a Matter node */
node::config_t node_config;
node_t *node = node::create(&node_config, app_attribute_update_cb, NULL);
@jonsmirl
Copy link
Contributor Author

jonsmirl commented Jul 29, 2022

Maybe like this, who is making the mandatory RootNode?

/* Create a Matter node */
node::config_t node_config;
node_t *node = node::create(&node_config, app_attribute_update_cb, NULL);

root_node::config_t root_node_config;
endpoint_t *endpoint = root_node::create(node, &root_node_config, ENDPOINT_FLAG_NONE);

on_off_switch::config_t switch_config;
endpoint_t *endpoint = on_off_switch::create(node, &switch_config, ENDPOINT_FLAG_NONE);

@chiragatal
Copy link
Contributor

Node here is more of a generic term. And when node::create() is called, it internally adds the mandatory root_node on endpoint 0.

@jonsmirl
Copy link
Contributor Author

Useful to explain the side effect of root_node creation via a comment in the example sources.

@chiragatal
Copy link
Contributor

Yeah, it makes sense, now that you mention it. It is easy to get confused between node and root_node.

@dhrishi dhrishi added the documentation Improvements or additions to documentation label Jul 29, 2022
@jonsmirl
Copy link
Contributor Author

Need to come up with different names for these various things...
BLE is using node too. Too many uses of the same word.

For a start...
node_t *node = node::get();
to
device_t *device = device::get();

Should that root_node be automatically created? How do you get a pointer to it to add optional clusters?

@chiragatal
Copy link
Contributor

chiragatal commented Aug 1, 2022

The root_node is being automatically created just to keep the application code simple and cover most of the use-cases. But if optional clusters are being added to root_node in a lot of use-cases, we can move the creation of root_node from the default node::create() and move that explicitly to the application.

With the current implementation, you can get the endpoint handle for the root node with this:

uint16_t root_node_endpoint_id = 0;
node_t *node = node::get();
endpoint_t *endpoint = endpoint::get(node, root_node_endpoint_id);

@dhrishi
Copy link
Collaborator

dhrishi commented Aug 25, 2022

Updated the comment in the application to reflect the creation of Root Node device type in create API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants