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

hawkbit-client: various simplifications #68

Merged

Conversation

Bastian-Krause
Copy link
Member

This is a collection of simplifications in the hawkbit-client module. Conflicts with the already open PRs should be minimal.

Note that this PR blocks the (to be created) hawkbit-client refactoring PR.

@Bastian-Krause Bastian-Krause force-pushed the bst/hawkbit-client-simplifications branch from 55000a3 to f3ea575 Compare January 5, 2021 11:42
@Bastian-Krause
Copy link
Member Author

Rebased on current master.

@Bastian-Krause Bastian-Krause force-pushed the bst/hawkbit-client-simplifications branch from f3ea575 to 2a1dfd2 Compare January 5, 2021 12:35
src/hawkbit-client.c Outdated Show resolved Hide resolved
src/hawkbit-client.c Outdated Show resolved Hide resolved
src/hawkbit-client.c Outdated Show resolved Hide resolved
@ejoerns ejoerns added the refactoring Refactors code label Jan 26, 2021
@Bastian-Krause Bastian-Krause mentioned this pull request Jan 26, 2021
@Bastian-Krause Bastian-Krause force-pushed the bst/hawkbit-client-simplifications branch from 2a1dfd2 to 62f7152 Compare January 27, 2021 13:47
@Bastian-Krause
Copy link
Member Author

Force-pushed integration of @ejoerns' feedback:

  • dropped accidental early return in get_binary()
  • clarified sha1sum argument documentation in get_binary()
  • use float values for average download speed in download_thread() again

@Bastian-Krause
Copy link
Member Author

Note: Failed CI testing requires the fix in #74 to be merged.

When asked about "status.result.progress" and its "of"/"cnt" fields, the
hawkBit maintainers answered:

  Back in the days the client API was designed first and hawkBit
  implemented the API. The idea of these fields was indeed to be able
  to report some kind of progress. However fields never made their way
  into server implementation. So long story short, client can send them
  as part of the feedback but hawkBit ignore them.

https://gitter.im/eclipse/hawkbit?at=5ddce9be55066245981a259e

So drop the construction of these JSON members as well as the
corresponding progress argument of json_build_status().

Signed-off-by: Bastian Krause <bst@pengutronix.de>
It is not necessary to pass the HTTP code obtained in get_binary()
along to calling functions. It is not used for anything other than log
messages outside of this function. So better log meaningful error
messages in get_binary() directly.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
The struct consists of the actual checksum result and the checksum type.
As the checksum type is hardcoded to sha1, there is no benefit from
passing this along in a struct.

For the RAUC usecase we do not rely on a checksum anyway as the bundle
signature is checked by RAUC. The only reason to check it that we can
determine if the transferred data is corrupt early, prior to triggering
the RAUC installation.

If the checksum type should be changed in the future, we could either
change the hardcoded style or even make it configurable via the config.
We do not need get_binary_checksum struct for neither case, so replace
it with the calculated checksum string directly.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
The md5 checksum is used nowhere.

For the RAUC usecase we do not rely on a checksum anyway as the bundle
signature is checked by RAUC. The only reason to check it that we can
determine if the transferred data is corrupt early, prior to triggering
the RAUC installation.

Currently the sha1 checksum is used, we can change that in the future
(see the previous commit removing the get_binary_checksum struct). Until
then we should not carry dead code.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
The base part of the URL is always the same. Thus construct the base URL
in build_api_url() and pass only relative paths extending the URL to it.

This simplifies build_api_url() calls in the whole module.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
…nt()

It is neither required nor a good idea to get the action ID from an
URL. It is also a bad idea to rely on the "c" GET parameter in
deployment base links. This is an implementation detail which can
change any time in hawkBit. It is not part of the DDI API, rather an
artifact from an example in the API's documentation.

So simply use the action ID from the deployment JSON response and drop
all corresponding regex matching code.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
…to download_thread()

Instead of calculating the speed ourselves, let's use curl's speed
calculation instead.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
@Bastian-Krause Bastian-Krause force-pushed the bst/hawkbit-client-simplifications branch from 62f7152 to 61c2ba8 Compare February 2, 2021 16:49
@Bastian-Krause
Copy link
Member Author

Rebased on current master.

@ejoerns ejoerns merged commit 91e9701 into rauc:master Feb 11, 2021
Bastian-Krause added a commit to Bastian-Krause/rauc-hawkbit-updater that referenced this pull request Mar 30, 2021
With the refactoring PRs rauc#63, rauc#64, rauc#65, rauc#66, rauc#68, rauc#71 and rauc#77, we gained
higher code quality. To keep it that way, introduce useful compiler
warnings for the QA_BUILD that reveal reintroduction of such issues.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
Bastian-Krause added a commit to Bastian-Krause/rauc-hawkbit-updater that referenced this pull request Mar 30, 2021
With the refactoring PRs rauc#63, rauc#64, rauc#65, rauc#66, rauc#68, rauc#71 and rauc#77, we gained
higher code quality. To keep it that way, introduce useful compiler
warnings for the QA_BUILD that reveal reintroduction of such issues.

Signed-off-by: Bastian Krause <bst@pengutronix.de>
@hmenn hmenn mentioned this pull request May 4, 2021
@Bastian-Krause Bastian-Krause deleted the bst/hawkbit-client-simplifications branch June 14, 2021 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactors code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants