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/04/09 18:20:34 UTC

[GitHub] [incubator-nuttx] gustavonihei opened a new pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

gustavonihei opened a new pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503


   ## Summary
   This PR intends to provide a fix for the multiple definition of config that are common to every ESP32-based board.
   
   ### Bug 
   ![image](https://user-images.githubusercontent.com/38959758/114223989-ded20b80-9946-11eb-9a9d-8d15eb505c2b.png)
   
   ### Fix 
   ![image](https://user-images.githubusercontent.com/38959758/114224008-e5f91980-9946-11eb-8462-2b090b11d14b.png)
   
   ## Impact
   No impact, just reorganization of current configuration.
   
   ## Testing
   CI build pass.
   


-- 
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 #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816928014


   They are common board configurations, this is why we have them with the boards.  Maybe the only one that could go to arch/esp32/Kconfig is the QEMU option.


-- 
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 edited a comment on pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816873903


   @davids5 for STM32 the common board Kconfig is included based only on `ARCH_CHIP_STM32` (https://github.com/apache/incubator-nuttx/blob/master/boards/Kconfig#L3161-L3163).  Isn't that affecting your custom boards?


-- 
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 #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816995598


   @davids5 that's actually a comment that was added, it does look similar to the first one, though.
   BTW, I see that the STM32 common Kconfig has a `BOARD_STM32_COMMON`, this is actually hiding all those options, so I think you won't see duplicates for a custom STM32 board.
   @gustavonihei this looks a bit similar to what we discussed.


-- 
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] xiaoxiang781216 merged pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503


   


-- 
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] davids5 commented on pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816948573


   with 10.0
   
   ![image](https://user-images.githubusercontent.com/1945821/114236176-197f7800-9936-11eb-93ac-58555ee1d763.png)
   
   
   With master I can see the duplicate, 
   
   ![image](https://user-images.githubusercontent.com/1945821/114235878-a83fc500-9935-11eb-92ef-53070e780c6e.png)
   


-- 
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] v01d commented on pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816923479


   I agree with the change, however I'm confused as to why there's a Kconfig under esp32/common. It looks like these options should be in esp32 arch's Kconfig as with every other arch does.


-- 
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] davids5 commented on pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816937145


   > @davids5 for STM32 the common board Kconfig is included based only on `ARCH_CHIP_STM32` (https://github.com/apache/incubator-nuttx/blob/master/boards/Kconfig#L3161-L3163). Isn't that affecting your custom boards?
   
   @Ouss4 I have not built with NuttX master in a while. On the next uptake I am sure there will be several issues to untangle. 


-- 
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 #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816873903


   @davids5 for STM32 the common board Kconfig is included based only on `ARCH_CHIP_STM32` (https://github.com/apache/incubator-nuttx/blob/master/boards/Kconfig#L3161-L3163).  Isn't that affecting some of your custom boards?


-- 
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 pull request #3503: ESP32: Fix multiple inclusion of the common Kconfig

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #3503:
URL: https://github.com/apache/incubator-nuttx/pull/3503#issuecomment-816926628


   > I agree with the change, however I'm confused as to why there's a Kconfig under esp32/common. It looks like these options should be in esp32 arch's Kconfig as with every other arch does.
   
   Placing these configs under arch may also work, but it will break the abstraction, after all it does not make much sense to enable File System or LCD options on the chip section.


-- 
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