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

mgmt: hawkbit: define attributes in the user application #69293

Conversation

maass-hamburg
Copy link
Collaborator

@maass-hamburg maass-hamburg commented Feb 21, 2024

Attributes can now be defined in the user application by the use of a callback function.
Resolves: #37640

image

@maass-hamburg
Copy link
Collaborator Author

ping

@maass-hamburg
Copy link
Collaborator Author

ping @ycsin

@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch from 327ed5d to 0ea9548 Compare March 14, 2024 08:42
@maass-hamburg
Copy link
Collaborator Author

just a rebase to the current main

@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch from 0ea9548 to c44fa72 Compare March 14, 2024 08:53
@ycsin
Copy link
Member

ycsin commented Mar 14, 2024

I haven't really looked at this in details yet but it seems to expose a little too much to the public than I would like, i.e. all the structs and stuff that used to be internal.

I wonder if it is possible to just have a mechanism for hawkBit to get the formatted json of the attributes from the app in the form of a buffer and sent it to the server?

@maass-hamburg
Copy link
Collaborator Author

I haven't really looked at this in details yet but it seems to expose a little too much to the public than I would like, i.e. all the structs and stuff that used to be internal.

they are needed if the user wants to define its own config callback. but maybe they could be put in a seperate header file and not in include/zephyr/mgmt/hawkbit.h maybe include/zephyr/mgmt/hawkbit_config.h or create a new folder include/zephyr/mgmt/hawkbit/ which contains include/zephyr/mgmt/hawkbit/hawkbit.h and include/zephyr/mgmt/hawkbit/config.h ?

I wonder if it is possible to just have a mechanism for hawkBit to get the formatted json of the attributes from the app in the form of a buffer and sent it to the server?

It might be, but then we can't use the json library to encode struct json_obj_descr json_cfg_descr, only for struct json_obj_descr json_cfg_data_descr. As it is currently not possible to add a already encoded object to a json_obj_descr.

@maass-hamburg
Copy link
Collaborator Author

I wonder if it is possible to just have a mechanism for hawkBit to get the formatted json of the attributes from the app in the form of a buffer and sent it to the server?

@ycsin with a change of the json lib it is possible

@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch 2 times, most recently from 8a86a83 to c5c25fb Compare March 14, 2024 12:53
@zephyrbot zephyrbot added the area: Samples Samples label Mar 14, 2024
@zephyrbot zephyrbot requested a review from kartben March 14, 2024 12:54
@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch 2 times, most recently from e2c94f4 to 54c8786 Compare March 15, 2024 06:48
@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch from 0eaadb1 to 0203fab Compare April 10, 2024 08:31
@maass-hamburg
Copy link
Collaborator Author

The approach in this PR should work, I just wonder if we can keep the default implementation simple (only send device_id) but provide more flexibility by having a mechanism (another step or public function, if enabled) to "merge" any custom attributes to the record in the server, i.e. the reason of last reboot (reset cause), die temp, stack watermark, ...

This problem is that the device attributes are only send to the server if the server requests it. This normally only happens once. the hawkBit server will only request it again, if you go in the user interface and click there on Request attributes update. It could be done to send it without the server requesting but that's outside of the scope of this PR .

@maass-hamburg maass-hamburg requested a review from ycsin April 10, 2024 08:42
Copy link
Member

@ycsin ycsin left a comment

Choose a reason for hiding this comment

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

This patch works - Have you considered doing this kind of one time configurations during hawkbit_init by extending it to accept an config struct? The same idea can also applies to #68806

@maass-hamburg
Copy link
Collaborator Author

This patch works - Have you considered doing this kind of one time configurations during hawkbit_init by extending it to accept an config struct? The same idea can also applies to #68806

I would prefer to not do it that way, as most subsystems in zephyr are also using this separate approach when it comes to setting callbacks.

ycsin
ycsin previously approved these changes Apr 13, 2024
Copy link
Member

@ycsin ycsin left a comment

Choose a reason for hiding this comment

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

The doc can use some improvements, LGTM in general

@maass-hamburg maass-hamburg requested a review from m-byte April 15, 2024 06:19
@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch 2 times, most recently from 92f5ed8 to 9bbb045 Compare April 15, 2024 09:21
@maass-hamburg maass-hamburg requested a review from ycsin April 15, 2024 11:15
@maass-hamburg maass-hamburg force-pushed the hawkbit_userdefined_attributes branch from 9bbb045 to 9264d08 Compare April 16, 2024 09:38
ycsin
ycsin previously approved these changes Apr 16, 2024
samples/subsys/mgmt/hawkbit/src/main.c Show resolved Hide resolved
Allows the attributes to be defined in the user application
by using a callback function.

Signed-off-by: Fin Maaß <[email protected]>
Extend sample for the custom attributes

Signed-off-by: Fin Maaß <[email protected]>
@maass-hamburg
Copy link
Collaborator Author

rebased to resolve conflicts

Copy link
Member

@ycsin ycsin left a comment

Choose a reason for hiding this comment

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

Re-ack

@maass-hamburg maass-hamburg requested review from jukkar and rlubos and removed request for m-byte April 17, 2024 18:25
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

LGTM

@carlescufi carlescufi merged commit d0be201 into zephyrproject-rtos:main Apr 17, 2024
21 checks passed
@maass-hamburg maass-hamburg deleted the hawkbit_userdefined_attributes branch April 29, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mgmt: hawkbit: allow user defintion of device attributes.
6 participants