[riot-notifications] [RIOT-OS/RIOT] dist/tools/openocd: add the possibility to do a reset before halt in openocd debug command (#8856)

Gaƫtan Harter notifications at github.com
Thu Apr 12 19:16:00 CEST 2018


cladmi requested changes on this pull request.

I would like to have a namespaced variable and homogenous in the way `OPENOCD_..._CMDS` work in flash.

> @@ -178,6 +178,13 @@ do_flash() {
 do_debug() {
     test_config
     test_elffile
+
+    # Configure halt command: prepend a reset if required by the board config
+    local _pre_reset_halt=''
+    if [ -n "${PRE_DEBUG_RESET_HALT}" ]; then

For me, the `PRE` here is wrong, because it changes the command from `halt` to `reset halt` it's not a previous command. Also, introducing a new really specific variable only for this specific way of handling the case seems bad to me.

> @@ -178,6 +178,13 @@ do_flash() {
 do_debug() {
     test_config
     test_elffile
+
+    # Configure halt command: prepend a reset if required by the board config
+    local _pre_reset_halt=''
+    if [ -n "${PRE_DEBUG_RESET_HALT}" ]; then

I would prefer the following

    # A meaningful comment on what this is :p
    : ${OPENOCD_EXTRA_INIT:=-c 'halt'}

And in the given board set

    export OPENOCD_DEBUG_HALT_CMD = -c 'reset halt'

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/8856#pullrequestreview-111709435
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20180412/0091ad21/attachment.html>


More information about the notifications mailing list