You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/02/08 21:27:58 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Ouss4 opened a new pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826
## Summary
This way we can use the `download` target with some flexibility on where the blobs are.
For example: `make download ESPTOOL_PORT=/dev/ttyUSB0 BLOBDIR=.`
## Impact
N/A
## Testing
esp32-devkitc
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r572390069
##########
File path: tools/esp32/Config.mk
##########
@@ -23,8 +23,8 @@
# archive. These replace the default definitions at tools/Config.mk
ifdef BLOBDIR
Review comment:
Do you have a better name? I couldn't come up with something better, It's just a directory that contains blobs :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573696307
##########
File path: tools/esp32/Config.mk
##########
@@ -22,12 +22,11 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
-else
- BOOTLOADER=$(IDF_PATH)/hello_world/build/bootloader/bootloader.bin
- PARTITION_TABLE=$(IDF_PATH)/hello_world/build/partition_table/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
+ PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
+ FLASH_BL=0x1000 $(BOOTLOADER)
+ FLASH_PT=0x8000 $(PARTITION_TABLE)
Review comment:
@Ouss4 I agree with @gustavonihei it is better to create: FLASH_BL_OFFSET=0x1000 and FLASH_PT_OFFSET=0x8000 and add it on final command.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573048572
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
```suggestion
BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
```
Just to make the style consistent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573059282
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
Will apply next time I push.
BTW, @gustavonihei I was thinking we should remove the part that flashes the binaries from a hello-world example. First it looks weird :) second, usually we flash the bootloader and partition-table only once and then we only need to re-flash the NuttX image.
So, will end up with `make download ESPTOOL_PORT` flashes only nuttx. `make download ESPTOOL_PORT=.. BLOBDIR=.` flashes all three.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#issuecomment-776310417
@gustavonihei @acassis PTAL again... I almost slept and forgot this one ;)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573713204
##########
File path: tools/esp32/Config.mk
##########
@@ -22,12 +22,11 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
-else
- BOOTLOADER=$(IDF_PATH)/hello_world/build/bootloader/bootloader.bin
- PARTITION_TABLE=$(IDF_PATH)/hello_world/build/partition_table/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
+ PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
+ FLASH_BL=0x1000 $(BOOTLOADER)
+ FLASH_PT=0x8000 $(PARTITION_TABLE)
Review comment:
I wouldn't say cosmetic, because in the proposed way FLASH_BT has now a dependency on another variable.
Being more explicit as @acassis suggested makes it easier to understand what is being passed to `esptool.py`.
Also, if the bootloader and partition table offsets are isolated, we can configure them with Kconfig later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r572388466
##########
File path: tools/esp32/Config.mk
##########
@@ -23,8 +23,8 @@
# archive. These replace the default definitions at tools/Config.mk
ifdef BLOBDIR
Review comment:
BLOBDIR could be renamed to something more intuitive.
Furthermore, it may be interesting to document what `BLOBDIR` intends to achieve.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis merged pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
acassis merged pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573760460
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
Ah, nice. Now I get why you decided to still keep the concatenated variable hehehe
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573756028
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
```
$(Q) if [ -z $(ESPTOOL_BINDIR) ]; then \
echo "DOWNLOAD error: Missing path to bootloader and partition table binary images."; \
echo "USAGE: make download ESPTOOL_BINDIR=<dir> ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573753820
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
Now `ESPTOOL_BINDIR` is required for the DOWNLOAD target.
It is important to add a verification for it and terminate in case it is not defined
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573082460
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
We have to update those tutorials as well to include these new modifications
I'll update this PR in a little while.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573048572
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
```suggestion
BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573059282
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
Will apply next time I push.
BTW, @gustavonihei I was thinking we should remove the part that gets the binaries from a hello-world example. First it looks weird :) second, usually we flash the bootloader and partition-table only once and then we only need to re-flash the NuttX image.
So, will end up with `make download ESPTOOL_PORT` flashes only nuttx. `make download ESPTOOL_PORT=.. BLOBDIR=.` flashes all three.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r572417020
##########
File path: tools/esp32/Config.mk
##########
@@ -23,8 +23,8 @@
# archive. These replace the default definitions at tools/Config.mk
ifdef BLOBDIR
Review comment:
I feel that calling it a BLOB gives the wrong impression that those are closed-source, which I think is not the case for the second stage bootloader and the partition table, right?
Maybe `ESPTOOL_BINDIR`, but I'm not also convinced about it, because BINDIR seems to relate to "install dir"...
It is no wonder that "naming things" is one of the most difficult things in computer science hehehe
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573758870
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
Part of this PR makes `ESPTOOL_BINDIR` not required for the download target. So I can do just `make downalod ESPTOOL_PORT=...` and flash only the NuttX image without the rest.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573732187
##########
File path: tools/esp32/Config.mk
##########
@@ -22,12 +22,11 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
-else
- BOOTLOADER=$(IDF_PATH)/hello_world/build/bootloader/bootloader.bin
- PARTITION_TABLE=$(IDF_PATH)/hello_world/build/partition_table/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
+ PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
+ FLASH_BL=0x1000 $(BOOTLOADER)
+ FLASH_PT=0x8000 $(PARTITION_TABLE)
Review comment:
I don't think we will ever change these configurations until we remove them all together.
Anyway, here goes: 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573753820
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
Now `ESPTOOL_BINDIR` is required for the DOWNLOAD target.
It is important to add a verification for it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573670992
##########
File path: tools/esp32/Config.mk
##########
@@ -22,12 +22,11 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
-else
- BOOTLOADER=$(IDF_PATH)/hello_world/build/bootloader/bootloader.bin
- PARTITION_TABLE=$(IDF_PATH)/hello_world/build/partition_table/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
+ PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
+ FLASH_BL=0x1000 $(BOOTLOADER)
+ FLASH_PT=0x8000 $(PARTITION_TABLE)
Review comment:
I don't know if concatenating those values in a variable really helps...
I would prefer keeping them separated and, in the future, turning the base address a new config to support different families in this same script.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r572654806
##########
File path: tools/esp32/Config.mk
##########
@@ -23,8 +23,8 @@
# archive. These replace the default definitions at tools/Config.mk
ifdef BLOBDIR
Review comment:
No, they are not closed source. They are part of IDF. Let's go with `ESPTOOL_BINDIR`, at least this is somewhat consistent with the other parameter (`ESPTOOL_PORT`)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573707850
##########
File path: tools/esp32/Config.mk
##########
@@ -22,12 +22,11 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
-else
- BOOTLOADER=$(IDF_PATH)/hello_world/build/bootloader/bootloader.bin
- PARTITION_TABLE=$(IDF_PATH)/hello_world/build/partition_table/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=$(ESPTOOL_BINDIR)/bootloader.bin
+ PARTITION_TABLE=$(ESPTOOL_BINDIR)/partition-table.bin
+ FLASH_BL=0x1000 $(BOOTLOADER)
+ FLASH_PT=0x8000 $(PARTITION_TABLE)
Review comment:
These are properties of the ROM bootloader, separating them would be just for cosmetic reasons.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573756028
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
```
USAGE="make download ESPTOOL_BINDIR=<dir> ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"
$(Q) if [ -z $(ESPTOOL_BINDIR) ]; then \
echo "DOWNLOAD error: Missing path to bootloader and partition table binary images."; \
echo "USAGE: $(USAGE)"; \
exit 1; \
fi
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573760460
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
Ah, nice. Now I get why you decided to still keep the concatenated variable hehehe
Sorry for the confusion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#issuecomment-775474580
FYI @gustavonihei
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573762893
##########
File path: tools/esp32/Config.mk
##########
@@ -80,5 +81,5 @@ define DOWNLOAD
echo "USAGE: make download ESPTOOL_PORT=<port> [ ESPTOOL_BAUD=<baud> ]"; \
exit 1; \
fi
Review comment:
I should update the USAGE line though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] saramonteiro commented on a change in pull request #2826: tools/esp32/Config.mk: Refine the usage of IDF binaries.
Posted by GitBox <gi...@apache.org>.
saramonteiro commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573646345
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
I am just waiting it to be merged to update my tutorials
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #2826: tools/esp32/Config.mk: Make the blob directory generic and not dependant on any board.
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #2826:
URL: https://github.com/apache/incubator-nuttx/pull/2826#discussion_r573067580
##########
File path: tools/esp32/Config.mk
##########
@@ -22,9 +22,9 @@
# and assemble source files and to insert the resulting object files into an
# archive. These replace the default definitions at tools/Config.mk
-ifdef BLOBDIR
- BOOTLOADER=${BLOBDIR}/esp32core/bootloader.bin
- PARTITION_TABLE=${BLOBDIR}/esp32core/partition-table.bin
+ifdef ESPTOOL_BINDIR
+ BOOTLOADER=${ESPTOOL_BINDIR}/bootloader.bin
+ PARTITION_TABLE=${ESPTOOL_BINDIR}/partition-table.bin
Review comment:
Good observation! In fact the bootloader and partition only need to be flashed once. Our documentation and Sara tutorials needs to me adapted to warning the user. He/she only need to do it one time!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org