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

net: lwm2m: Add observe callback for observe and notification events #39651

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

MaikVermeulen
Copy link
Contributor

@MaikVermeulen MaikVermeulen commented Oct 22, 2021

Added an observe callback so that the application can register to
receive events like observer added/deleted, and notification acked/
timed out. The notifications can be traced back to the exact data
contained within them by use of the user_data pointer.

Fixes #38531.

Signed-off-by: Maik Vermeulen maik.vermeulen@innotractor.com

@github-actions github-actions bot added area: API Changes to public APIs area: Networking labels Oct 22, 2021
@MaikVermeulen MaikVermeulen force-pushed the lwm2m_observe_cb branch 5 times, most recently from 6abb175 to 6b38ba9 Compare October 23, 2021 10:14
include/net/lwm2m.h Outdated Show resolved Hide resolved
include/net/lwm2m.h Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved
subsys/net/lib/lwm2m/lwm2m_engine.c Outdated Show resolved Hide resolved
int send_traceable_notification(struct lwm2m_ctx *client_ctx, struct lwm2m_obj_path *path,
void *user_data)
{
/* TODO This should generate the notifications in place and not use the resource buffers,
Copy link
Contributor

Choose a reason for hiding this comment

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

So, where are we with this? The observe notification mechanism looks pretty ok to me already, and it's pretty independent (it doesn't really matter what data do we provide through the user_data pointer as long as we're consistent), so perhaps the PR should be split in two parts, as the observe events part looks quite mergable as it is now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for checking in Robert! I agree it's better to split it up.

As for the proper historical notification mechanism, I haven't had the chance to really dive into yet. Our backend is still catching up with the current changes, so implementing the historical notifications / SenML JSON is not scheduled yet.

@MaikVermeulen MaikVermeulen force-pushed the lwm2m_observe_cb branch 2 times, most recently from 2282df8 to bbbd913 Compare October 27, 2021 16:03
@MaikVermeulen MaikVermeulen marked this pull request as ready for review November 1, 2021 09:45
Copy link
Contributor

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Looks good, but it seems that we're missing a small update of the lwm2m_client sample, see the CI results:

/workdir/zephyr/samples/net/lwm2m_client/src/lwm2m-client.c: In function 'main':
/workdir/zephyr/samples/net/lwm2m_client/src/lwm2m-client.c:502:2: error: too few arguments to function 'lwm2m_rd_client_start'
  502 |  lwm2m_rd_client_start(&client, CONFIG_BOARD, flags, rd_client_event);

@MaikVermeulen
Copy link
Contributor Author

MaikVermeulen commented Nov 18, 2021

Seems like I messed up the rebase, but it's fixed now.
Unfortunately a whole a bunch of reviewers is added now... Sorry folks!

EDIT: One of the checks is failing due to an internal server error 500..?

@rlubos
Copy link
Contributor

rlubos commented Nov 19, 2021

Hmm but I still don't see the changes in the lwm2m_client sample?

@aescolar aescolar removed their request for review November 21, 2021 09:08
@MaikVermeulen MaikVermeulen force-pushed the lwm2m_observe_cb branch 3 times, most recently from 7da15fc to 58665e1 Compare November 24, 2021 19:30
@github-actions github-actions bot added the area: Samples Samples label Nov 24, 2021
Added an observe callback so that the application can register to
receive events like observer added/deleted, and notification acked/
timed out. The notifications can be traced back to the exact data
contained within them by use of the user_data pointer.

Fixes zephyrproject-rtos#38531.

Signed-off-by: Maik Vermeulen <maik.vermeulen@innotractor.com>
Copy link
Contributor

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

Thank you

@nashif nashif merged commit f2ca6a8 into zephyrproject-rtos:main Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: ARC ARC Architecture area: ARM ARM (32-bit) Architecture area: ARM64 ARM (64-bit) Architecture area: Boards area: Devicetree area: Kconfig area: native port Host native arch port (native_sim) area: Networking area: Samples Samples area: X86 x86 Architecture (32-bit) area: Xtensa Xtensa Architecture
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lwm2m: report acknowledgement of notifications to app
5 participants