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

nrf_rpc: call bound_handler after init reply #1550

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

Damian-Nordic
Copy link
Contributor

  1. Ensure that the group's bound_handler is called after sending the init reply. This is to make sure that the remote knows our group ID and is able to process incoming commands when the handler is called.
  2. Similarly, signal groups_init_event after sending the init reply.

nrf_rpc: refactor internal tasks

  1. Rename internal_data to internal_task to better describe the purpose of the structure.
  2. Use internal_task_consumed event to notify about freeing the global task object for all types of tasks.
  3. Use union for different types of tasks to not bloat the internal task structure when more tasks or fields are added.

* initialization from within the task to ensure that when this happens the init reply has
* already been sent and the remote is ready to receive nRF RPC commands.
*/
if (((*group)->bound_handler != NULL) || send_reply || signal_groups_init_event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that this will not result in multiple bound_handler calls?

Copy link
Contributor Author

@Damian-Nordic Damian-Nordic Oct 31, 2024

Choose a reason for hiding this comment

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

I am not, but previously it was the same, right? I mean bound_handler was called for each incoming init packet, reglardless if this was the first or n-th packet.

Copy link
Contributor

@alxelax alxelax left a comment

Choose a reason for hiding this comment

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

I like these changes. The solution in general looks more solid. Well done.

@anhmolt

This comment was marked as resolved.

@anhmolt anhmolt closed this Nov 4, 2024
@Damian-Nordic

This comment was marked as outdated.

@anhmolt anhmolt reopened this Nov 4, 2024
@anhmolt

This comment was marked as outdated.

@Damian-Nordic
Copy link
Contributor Author

@doki-nordic Could you review this PR?

1. Rename internal_data to internal_task to better describe
   the purpose of the structure.
2. Use internal_task_consumed event to notify about freeing
   the global task object for all types of tasks.
3. Use union for different types of tasks to not bloat the
   internal task structure when more tasks or fields are
   added.

Signed-off-by: Damian Krolik <[email protected]>
1. Ensure that the group's bound_handler is called after
   sending the init reply. This is to make sure that the
   remote knows our group ID and is able to process
   incoming commands when the handler is called.
2. Similarly, signal groups_init_event after sending the
   init reply.

Signed-off-by: Damian Krolik <[email protected]>
@rlubos rlubos merged commit 4f3ed25 into nrfconnect:main Nov 18, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants