-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: autohandler and shell optimize #71037
base: main
Are you sure you want to change the base?
mgmt: hawkbit: autohandler and shell optimize #71037
Conversation
b3e2f2d
to
34ccf08
Compare
34ccf08
to
839b1c2
Compare
839b1c2
to
28ce2f7
Compare
28ce2f7
to
4755f4d
Compare
3a04bdf
to
d7fcf8c
Compare
d7fcf8c
to
54b5308
Compare
01155a7
to
12df207
Compare
12df207
to
9803ac3
Compare
f423451
to
2b559e2
Compare
2b559e2
to
55c4cdf
Compare
Bulk changing the milestone from everything which did not make it to the v3.7.0 freeze deadline Please note that until rc2 only PRs containing bug fixes, docs and tests can be merged. Exceptions require approval by the release team and a vote by the TSC. |
18b454b
to
c6cc5a6
Compare
I like to get forward with this PR.
Both the shell and the autohandler are a great at making hawkBit easy to use and accessible. That being said, I hope we can get this PR approved and merged soon, so we can finally be able to get to #70787 |
c6cc5a6
to
c328fac
Compare
rebased to resolve conflicts |
@ycsin are you coming back for a review? |
staticly init hawkbit_work_handle Signed-off-by: Fin Maaß <[email protected]>
add option for autohandler to only run once. Signed-off-by: Fin Maaß <[email protected]>
Add hawkbit_autohandler_wait() to be able to wait for the autohandler to finish. Signed-off-by: Fin Maaß <[email protected]>
Also use a workqueue, when execution of hawkBit is requested via shell. Signed-off-by: Fin Maaß <[email protected]>
If the run of the autohandler is started from shell, it will be logged. Signed-off-by: Fin Maaß <[email protected]>
Be able to delay the next run of the hawkbit autohandler. Signed-off-by: Fin Maaß <[email protected]>
Mention change of hawkbit autohandler and shell in the migration guide and the release notes. Signed-off-by: Fin Maaß <[email protected]>
Add function to get the poll interval. This is needed to seperate the autohandler from the main hawkbit code. Signed-off-by: Fin Maaß <[email protected]>
seperate the autohandler from the main hawkbit source. This way the autohandler can be disabled if it is not needed. Signed-off-by: Fin Maaß <[email protected]>
seperate the hawkbit header files, to make it clearer. Signed-off-by: Fin Maaß <[email protected]>
move HAWKBIT_JSON_URL out of the header. Signed-off-by: Fin Maaß <[email protected]>
Mention change of the hawkbit header files. Signed-off-by: Fin Maaß <[email protected]>
c328fac
to
5c0c575
Compare
rebased due to unrelated fails in the ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ycsin are you coming back for a review?
Had another look but there seems to be nothing new to review, I'd say my previous review is still up-to-date
First of all as a Maintainer I want to say that:
- the autohandler and the hawkBit shell will stay.
- useful additions to both of them are welcome.
Both the shell and the autohandler are a great at making hawkBit easy to use and accessible. Not everybody want to write their own autohandler and therefore use the autohandler, which will just work and you don't have to do much yourself. And fore the ones, who don't want to use this autohandler, they can, if this PR gets merged, deactivate the included autohandler.
I agree that the bundled autohandler in this PR might be useful, I have no qualms if this PR updated the one in the samples to make it more portable, which IMHO serves pretty much the same purpose to show the users how to build one, instead of forcing an implementation on them.
I've had my custom implementation of the autohandler in the past, had I chose to upstream and developed it around my use case, folks that came after me probably have to either use it my way or go away, I think that would make hawkBit less inviting
config HAWKBIT_AUTOHANDLER | ||
bool "hawkBit autohandler" | ||
select EVENTS | ||
default y | ||
help | ||
Activate autohandler to handle the update process automatically. | ||
|
||
config HAWKBIT_SHELL | ||
bool "hawkBit shell utilities" | ||
depends on SHELL | ||
depends on HAWKBIT_AUTOHANDLER |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And fore the ones, who don't want to use this autohandler, they can, if this PR gets merged, deactivate the included autohandler.
Does this dependency means that for user who doesn't want to use the bundled autohandler, they lose access to shell for updating the configurations as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well yes at least when they don't activate the Kconfig for the autohandler. They could still choose to include the autohandler and don't use it theirself.
@ycsin As I said before the autohandler and the shell are going to stay and if there are good reasons, like here, they are also going to be extended. Requested changes in the other way are hereby dismissed. You can follow the escalation process if you are not okay with that, otherwise I expect a review approving this or requesting changes that are not on this fundamental level. |
see coments above, he also removed himself from the colaborators of hawkbit.
also use the workqueue for the run of hawkBit, when requested via the shell. This way the run via the shell will use the same thread as the autohandler run. We also can eliminate redundant code.
This has mainly the benefit of not having to extend the stack size of the shell threads. Explicitly when having multiple shell instances, then the stack of every instance has to be bigger.
This is needed for #70787 as it moves the buffers from global to the stack.