tools/openocd: check if variable for extra reset_config is set#12157
Merged
fjmolinas merged 1 commit intoRIOT-OS:masterfrom Sep 3, 2019
Merged
Conversation
aabadie
added
the
Type: cleanup
Contributor
|
Tested this fixes the issue master: PR |
fjmolinas
added
CI: ready for build
Contributor
I don't have an argument against or in favor of any of the solutions. @cladmi do you have a preference? |
aabadie
changed the title
Contributor
|
Good catch. We do not need to check for it being an integer, matching to a string also works and would remove the warning on an empty value: diff --git a/dist/tools/openocd/openocd.sh b/dist/tools/openocd/openocd.sh
index 3a73038e0..e6776bbee 100755
--- a/dist/tools/openocd/openocd.sh
+++ b/dist/tools/openocd/openocd.sh
@@ -121,8 +121,7 @@
# The single quotes are important on the line above, or it will not work.
# Handle OPENOCD_RESET_USE_CONNECT_ASSERT_SRST
-if ([ ! -z "${OPENOCD_RESET_USE_CONNECT_ASSERT_SRST}" ] && \
- [ "${OPENOCD_RESET_USE_CONNECT_ASSERT_SRST}" -eq 1 ]); then
+if [ "${OPENOCD_RESET_USE_CONNECT_ASSERT_SRST}" = "1" ]; then
OPENOCD_EXTRA_RESET_INIT+="-c 'reset_config connect_assert_srst'"
fi
|
This avoids the following warning message when running OpenOCD: 'openocd.sh: line 124: [: : integer expression expected'
aabadie
force-pushed
the
pr/tools/openocd_default_connect_assert_srst_val
branch
from
99d155d to
9065f83
Compare
Contributor
Author
Indeed much better solution! I amended the commit with it. |
Contributor
|
Good, it works without warning: nucleo-f091rc still uses connect_assert_srstNo warning on other boards, like `iotlab-m3` |
Contributor
And using `OPENOCD_RESET_USE_CONNECT_ASSERT_SRST=0` still disables using `connect_assert_srst` on `nucleo-f091rc` |
fjmolinas
added
the
Reviewed: 4-code-style
fjmolinas
approved these changes
Sep 3, 2019
Contributor
fjmolinas
left a comment
fjmolinas
left a comment
There was a problem hiding this comment.
Reproduced the testing procedure described, same result as before. ACK!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
This PR fixes a warning message raised by OpenOCD when flashing without
OPENOCD_RESET_USE_CONNECT_ASSERT_SRST = 1so on almost all boards flashed by OpenOCD exceptnucleo-f091rc.The message is:
openocd.sh: line 124: [: : integer expression expectedThe fix is simply to also check if the variable is set.
Another solution could be to assign a default value of
0, e.g. here. I don't know what's best.Testing procedure
Issues/PRs references
Cleanup of #11976