You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2023/01/07 01:45:12 UTC

[GitHub] [tvm] mehrdadh opened a new pull request, #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

mehrdadh opened a new pull request, #13723:
URL: https://github.com/apache/tvm/pull/13723

   This PR adds serial number to flash command (nrfjprog) to fix cases where multiple devices are available.
   
   cc @gromero 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mehrdadh commented on a diff in pull request #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065034428


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -262,7 +262,11 @@ def _get_openocd_device_args(serial_number: str = None):
     return ["--serial", generic_find_serial_port(serial_number)]
 
 
-def _get_nrf_device_args(serial_number: str = None):
+def _get_nrf_device_args(serial_number: str = None) -> list:
+    # iSerial has string type which could mistmatch with
+    # the output of `nrfjprog --ids`. Example: 001050007848 vs 1050007848
+    serial_number = str(int(serial_number))

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #13723: [microTVM][Zephyr] Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065038908


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -714,23 +716,27 @@ def flash(self, options):
         if _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME):
             return  # NOTE: qemu requires no flash step--it is launched from open_transport.
 
+        flash_runner = _get_flash_runner()
         # The nRF5340DK requires an additional `nrfjprog --recover` before each flash cycle.
         # This is because readback protection is enabled by default when this device is flashed.
         # Otherwise, flashing may fail with an error such as the following:
         #  ERROR: The operation attempted is unavailable due to readback protection in
         #  ERROR: your device. Please use --recover to unlock the device.
         zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
-        if zephyr_board.startswith("nrf5340dk") and _get_flash_runner() == "nrfjprog":
+        if zephyr_board.startswith("nrf5340dk") and flash_runner == "nrfjprog":
             recover_args = ["nrfjprog", "--recover"]
             recover_args.extend(_get_nrf_device_args(serial_number))
             check_call(recover_args, cwd=API_SERVER_DIR / "build")
 
         flash_extra_args = []
-        if _get_flash_runner() == "openocd" and serial_number:
-            flash_extra_args = ["--cmd-pre-init", f"""hla_serial {serial_number}"""]
+        if flash_runner == "openocd" and serial_number:
+            flash_extra_args += ["--cmd-pre-init", f"""hla_serial {serial_number}"""]

Review Comment:
   @mehrdadh I wonder if it would be `f'"hla_serial {serial_number}"'`, because  afaics the triple double-quote in f-string won't expand to an additional double quote -- as I think @guberti pointed out, but I also think it's necessary one double-quite in the final string for the command as you said  / experimented. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mehrdadh commented on a diff in pull request #13723: [microTVM][Zephyr] Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065041132


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -714,23 +716,27 @@ def flash(self, options):
         if _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME):
             return  # NOTE: qemu requires no flash step--it is launched from open_transport.
 
+        flash_runner = _get_flash_runner()
         # The nRF5340DK requires an additional `nrfjprog --recover` before each flash cycle.
         # This is because readback protection is enabled by default when this device is flashed.
         # Otherwise, flashing may fail with an error such as the following:
         #  ERROR: The operation attempted is unavailable due to readback protection in
         #  ERROR: your device. Please use --recover to unlock the device.
         zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
-        if zephyr_board.startswith("nrf5340dk") and _get_flash_runner() == "nrfjprog":
+        if zephyr_board.startswith("nrf5340dk") and flash_runner == "nrfjprog":
             recover_args = ["nrfjprog", "--recover"]
             recover_args.extend(_get_nrf_device_args(serial_number))
             check_call(recover_args, cwd=API_SERVER_DIR / "build")
 
         flash_extra_args = []
-        if _get_flash_runner() == "openocd" and serial_number:
-            flash_extra_args = ["--cmd-pre-init", f"""hla_serial {serial_number}"""]
+        if flash_runner == "openocd" and serial_number:
+            flash_extra_args += ["--cmd-pre-init", f"""hla_serial {serial_number}"""]

Review Comment:
   I will test this again and fix it in a follow up PR if that was the case



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gromero commented on a diff in pull request #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
gromero commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065038908


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -714,23 +716,27 @@ def flash(self, options):
         if _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME):
             return  # NOTE: qemu requires no flash step--it is launched from open_transport.
 
+        flash_runner = _get_flash_runner()
         # The nRF5340DK requires an additional `nrfjprog --recover` before each flash cycle.
         # This is because readback protection is enabled by default when this device is flashed.
         # Otherwise, flashing may fail with an error such as the following:
         #  ERROR: The operation attempted is unavailable due to readback protection in
         #  ERROR: your device. Please use --recover to unlock the device.
         zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
-        if zephyr_board.startswith("nrf5340dk") and _get_flash_runner() == "nrfjprog":
+        if zephyr_board.startswith("nrf5340dk") and flash_runner == "nrfjprog":
             recover_args = ["nrfjprog", "--recover"]
             recover_args.extend(_get_nrf_device_args(serial_number))
             check_call(recover_args, cwd=API_SERVER_DIR / "build")
 
         flash_extra_args = []
-        if _get_flash_runner() == "openocd" and serial_number:
-            flash_extra_args = ["--cmd-pre-init", f"""hla_serial {serial_number}"""]
+        if flash_runner == "openocd" and serial_number:
+            flash_extra_args += ["--cmd-pre-init", f"""hla_serial {serial_number}"""]

Review Comment:
   @mehrdadh I wonder if it would be `f'"hla_serial {serial_number}"'`, because  afaics the triple double-quote in f-string won't expand to an additional double quote, but I think it's necessary one double-quite in the final string for the command as you said  / experimented. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] guberti commented on a diff in pull request #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
guberti commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065020364


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -714,23 +716,27 @@ def flash(self, options):
         if _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME):
             return  # NOTE: qemu requires no flash step--it is launched from open_transport.
 
+        flash_runner = _get_flash_runner()
         # The nRF5340DK requires an additional `nrfjprog --recover` before each flash cycle.
         # This is because readback protection is enabled by default when this device is flashed.
         # Otherwise, flashing may fail with an error such as the following:
         #  ERROR: The operation attempted is unavailable due to readback protection in
         #  ERROR: your device. Please use --recover to unlock the device.
         zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
-        if zephyr_board.startswith("nrf5340dk") and _get_flash_runner() == "nrfjprog":
+        if zephyr_board.startswith("nrf5340dk") and flash_runner == "nrfjprog":
             recover_args = ["nrfjprog", "--recover"]
             recover_args.extend(_get_nrf_device_args(serial_number))
             check_call(recover_args, cwd=API_SERVER_DIR / "build")
 
         flash_extra_args = []
-        if _get_flash_runner() == "openocd" and serial_number:
-            flash_extra_args = ["--cmd-pre-init", f"""hla_serial {serial_number}"""]
+        if flash_runner == "openocd" and serial_number:
+            flash_extra_args += ["--cmd-pre-init", f"""hla_serial {serial_number}"""]

Review Comment:
   nit: 
   ```suggestion
               flash_extra_args += ["--cmd-pre-init", f"hla_serial {serial_number}"]
   ```



##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -262,7 +262,11 @@ def _get_openocd_device_args(serial_number: str = None):
     return ["--serial", generic_find_serial_port(serial_number)]
 
 
-def _get_nrf_device_args(serial_number: str = None):
+def _get_nrf_device_args(serial_number: str = None) -> list:
+    # iSerial has string type which could mistmatch with
+    # the output of `nrfjprog --ids`. Example: 001050007848 vs 1050007848
+    serial_number = str(int(serial_number))

Review Comment:
   Would it be cleaner to do this? Not sure.
   ```suggestion
       serial_number = serial_number.lstrip("0")
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] tvm-bot commented on pull request #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13723:
URL: https://github.com/apache/tvm/pull/13723#issuecomment-1374342019

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * cc @alanmacd, @gromero, @leandron, @mkatanbaf <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mehrdadh commented on a diff in pull request #13723: [microTVM][Zephyr]Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
mehrdadh commented on code in PR #13723:
URL: https://github.com/apache/tvm/pull/13723#discussion_r1065032575


##########
apps/microtvm/zephyr/template_project/microtvm_api_server.py:
##########
@@ -714,23 +716,27 @@ def flash(self, options):
         if _find_platform_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME):
             return  # NOTE: qemu requires no flash step--it is launched from open_transport.
 
+        flash_runner = _get_flash_runner()
         # The nRF5340DK requires an additional `nrfjprog --recover` before each flash cycle.
         # This is because readback protection is enabled by default when this device is flashed.
         # Otherwise, flashing may fail with an error such as the following:
         #  ERROR: The operation attempted is unavailable due to readback protection in
         #  ERROR: your device. Please use --recover to unlock the device.
         zephyr_board = _find_board_from_cmake_file(API_SERVER_DIR / CMAKELIST_FILENAME)
-        if zephyr_board.startswith("nrf5340dk") and _get_flash_runner() == "nrfjprog":
+        if zephyr_board.startswith("nrf5340dk") and flash_runner == "nrfjprog":
             recover_args = ["nrfjprog", "--recover"]
             recover_args.extend(_get_nrf_device_args(serial_number))
             check_call(recover_args, cwd=API_SERVER_DIR / "build")
 
         flash_extra_args = []
-        if _get_flash_runner() == "openocd" and serial_number:
-            flash_extra_args = ["--cmd-pre-init", f"""hla_serial {serial_number}"""]
+        if flash_runner == "openocd" and serial_number:
+            flash_extra_args += ["--cmd-pre-init", f"""hla_serial {serial_number}"""]

Review Comment:
   I remember testing this without "" around the `hla_serial` option and it was not working



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] mehrdadh merged pull request #13723: [microTVM][Zephyr] Fix flash command for nrfjprog

Posted by GitBox <gi...@apache.org>.
mehrdadh merged PR #13723:
URL: https://github.com/apache/tvm/pull/13723


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org