Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logic for generic platform #553

Merged

Conversation

k7g03z
Copy link
Contributor

@k7g03z k7g03z commented Jul 28, 2022

This PR fixes the incorrect logic in checking if the generic platform is being used.

#550

Signed-off-by: Kevin Goez [email protected]

@k7g03z k7g03z requested a review from pablogs9 as a code owner July 28, 2022 13:33
@pablogs9 pablogs9 requested a review from Acuadros95 July 28, 2022 13:51
@pablogs9
Copy link
Member

Please check the CI

@Acuadros95
Copy link
Contributor

Acuadros95 commented Jul 28, 2022

That check is a little bit misleading.

As you can see here, if $PLATFORM == generic, the sourced file is exactly the same as $PREFIX/config/$RTOS/$PLATFORM/build.sh == $PREFIX/config/$RTOS/generic/build.sh:

if [ $PLATFORM != "generic" ] && [ -d "$PREFIX/config/$RTOS/generic" ]; then
    . $PREFIX/config/$RTOS/generic/build.sh
else
    . $PREFIX/config/$RTOS/$PLATFORM/build.sh
fi

If you take a look at each RTOS folder, if the generic folder is present it should always be used. So we can just take that check out and use:

if [ -d "$PREFIX/config/$RTOS/generic" ]; then
    . $PREFIX/config/$RTOS/generic/build.sh
else
    . $PREFIX/config/$RTOS/$PLATFORM/build.sh
fi

@k7g03z
Copy link
Contributor Author

k7g03z commented Jul 28, 2022

If you take a look at each RTOS folder, if the generic folder is present it should always be used. So we can just take that check out and use:

if [ -d "$PREFIX/config/$RTOS/generic" ]; then
    . $PREFIX/config/$RTOS/generic/build.sh
else
    . $PREFIX/config/$RTOS/$PLATFORM/build.sh
fi

@Acuadros95
Why would you always use the generic if the user selects a specific platform?

I'm not exactly sure what the logic is behind the generic... After reading the comment above, I can imagine that some platforms do not have a PLATFORM-specific build script and therefore should use the generic script...

Would the following logic work?

if [ -f "$PREFIX/config/$RTOS/$PLATFORM/build.sh" ]; then
    . $PREFIX/config/$RTOS/$PLATFORM/build.sh
elif [ -f "$PREFIX/config/$RTOS/generic/build.sh" ]; then
    . $PREFIX/config/$RTOS/generic/build.sh
else 
   ERROR!
fi

@Acuadros95
Copy link
Contributor

@k7g03z This repo gathers different RTOSes and boards with different configuration systems.

For example, freertos uses a folder for each supported platform, as the configuration, flash or build process are different for each board: detail.

But for zephyr, the different platforms are handle on the same generic folder: detail.

I understand its confusing, but this repository is being replaced by the Standalone build system tools and we will avoid going on deep changes for now.

@k7g03z k7g03z force-pushed the fix/broken_logic_for_lib_generation branch from 28edd77 to 0ff7643 Compare July 29, 2022 13:36
Signed-off-by: Kevin Goez <[email protected]>
@k7g03z k7g03z force-pushed the fix/broken_logic_for_lib_generation branch from 0ff7643 to 1f34a12 Compare July 29, 2022 13:37
@k7g03z
Copy link
Contributor Author

k7g03z commented Jul 29, 2022

@k7g03z This repo gathers different RTOSes and boards with different configuration systems.

For example, freertos uses a folder for each supported platform, as the configuration, flash or build process are different for each board: detail.

But for zephyr, the different platforms are handle on the same generic folder: detail.

I understand its confusing, but this repository is being replaced by the Standalone build system tools and we will avoid going on deep changes for now.

Based on your feedback, the check $PLATFORM != generic seems to be unnecessary in general as well as confusing. I've taken it out in the latest push.

@Acuadros95 Acuadros95 merged commit b81290a into micro-ROS:main Aug 16, 2022
@Acuadros95
Copy link
Contributor

@mergify backport humble galactic foxy

mergify bot pushed a commit that referenced this pull request Aug 16, 2022
Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)
mergify bot pushed a commit that referenced this pull request Aug 16, 2022
Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)
mergify bot pushed a commit that referenced this pull request Aug 16, 2022
Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)
@mergify
Copy link
Contributor

mergify bot commented Aug 16, 2022

backport humble galactic foxy

✅ Backports have been created

Acuadros95 pushed a commit that referenced this pull request Aug 16, 2022
Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)

Co-authored-by: k7g03z <[email protected]>
@k7g03z k7g03z deleted the fix/broken_logic_for_lib_generation branch August 16, 2022 11:59
Acuadros95 pushed a commit that referenced this pull request Aug 17, 2022
Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)

Co-authored-by: k7g03z <[email protected]>
Acuadros95 added a commit that referenced this pull request Aug 17, 2022
* Remove unnessary check (#553)

Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)

* Update cmake fix

Signed-off-by: acuadros95 <[email protected]>

Signed-off-by: acuadros95 <[email protected]>
Co-authored-by: k7g03z <[email protected]>
Co-authored-by: acuadros95 <[email protected]>
nomumu added a commit to nomumu/micro_ros_setup that referenced this pull request Sep 12, 2022
* Update yaml_filter.py (micro-ROS#365) (micro-ROS#367)

(cherry picked from commit ce41dcb)

Co-authored-by: Pablo Garrido <[email protected]>

* Support for flashing Olimex STM32-E407 with STLINK v2 debuggers (micro-ROS#374)

* Update standalone build tools section (backport micro-ROS#380) (micro-ROS#382)

* Update standalone build tools section (micro-ROS#380)

* Update standalone build tools section

* Upd

* Updates

(cherry picked from commit e13e64d)

# Conflicts:
#	README.md

* Update

Co-authored-by: Pablo Garrido <[email protected]>

* Simplify micro-ROS Agent build (micro-ROS#378) (micro-ROS#384)

* Simplify agent build

* Revert this

* Update config/agent_uros_packages.repos

Co-authored-by: Pablo Garrido <[email protected]>

Co-authored-by: Antonio Cuadros <[email protected]>
(cherry picked from commit 70ccf66)

Co-authored-by: Pablo Garrido <[email protected]>

* Update README.md (micro-ROS#386) (micro-ROS#387)

(cherry picked from commit a4c138e)

Co-authored-by: Pablo Garrido <[email protected]>

* Add renesas RA6M5 (backport micro-ROS#390) (micro-ROS#391)

* Add renesas RA6M5 (micro-ROS#390)

* Add renesas RA6M5

* Fix

* Avoid toolchain

(cherry picked from commit d27b83e)

# Conflicts:
#	README.md

* Fix

Co-authored-by: Pablo Garrido <[email protected]>

* openocd: added script to reset devices (micro-ROS#399)

* fix: use FW_TARGETDIR instead of firmware (micro-ROS#400)

* Temporal disable Rolling Agent CI (backport micro-ROS#408) (micro-ROS#410)

* Temporal disable Rolling Agent CI (micro-ROS#408)

(cherry picked from commit 218170b)

# Conflicts:
#	.github/workflows/nightly.yml

* Update nightly.yml

Co-authored-by: Pablo Garrido <[email protected]>

* Remove unnecessary rosdep (micro-ROS#411) (micro-ROS#412)

(cherry picked from commit c03d66e)

Co-authored-by: Pablo Garrido <[email protected]>

* Fix rosdep (backport micro-ROS#414) (micro-ROS#415)

* Fix rosdep (micro-ROS#414)

* Fix rosdep

* Update

(cherry picked from commit 3a28e75)

# Conflicts:
#	scripts/create_firmware_ws.sh

* Fix conflicts

Co-authored-by: Pablo Garrido <[email protected]>

* Modify host build (micro-ROS#418) (micro-ROS#421)

(cherry picked from commit 97f013e)

Co-authored-by: Pablo Garrido <[email protected]>

* Skip existing repos in vcs (micro-ROS#419) (micro-ROS#423)

(cherry picked from commit ecdd1ab)

Co-authored-by: Pablo Garrido <[email protected]>

* Fix host src folder (micro-ROS#417) (micro-ROS#425)

(cherry picked from commit fc7faa3)

Co-authored-by: Pablo Garrido <[email protected]>

* Build dynamic lib of rosidl_typesupport _microxrcedds (backport micro-ROS#473) (micro-ROS#475)

* build dynamic lib of rosidl_typesupport of (micro-ROS#473)

microxrcedds_c

(cherry picked from commit 3136ad0)

# Conflicts:
#	config/host/generic/build.sh
#	config/host/generic/client_host_packages.repos

* Conflicts

Signed-off-by: Pablo Garrido <[email protected]>

Co-authored-by: VictorLee <[email protected]>
Co-authored-by: Pablo Garrido <[email protected]>

* Fix Zephyr CI (backport micro-ROS#480) (micro-ROS#482)

* Fix Zephyr CI (micro-ROS#480)

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit 0942f74)

# Conflicts:
#	config/zephyr/generic/create.sh

* Update

Signed-off-by: Pablo Garrido <[email protected]>

Co-authored-by: Pablo Garrido <[email protected]>

* Update changelog

Signed-off-by: Pablo Garrido <[email protected]>

* 1.0.0

* Enable rolling agent (backport micro-ROS#479) (micro-ROS#484)

* Enable rolling agent (micro-ROS#479)

* Enable rolling agent

Signed-off-by: Pablo Garrido <[email protected]>

* Install python3.9-dev

Signed-off-by: Pablo Garrido <[email protected]>

* Fix

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit 91ed371)

# Conflicts:
#	.github/workflows/ci.yml
#	.github/workflows/nightly.yml

* update

Signed-off-by: Pablo Garrido <[email protected]>

Co-authored-by: Pablo Garrido <[email protected]>

* Fix CI

Signed-off-by: Pablo Garrido <[email protected]>

* Fix CI

Signed-off-by: Pablo Garrido <[email protected]>

* Fix Agent and host Rolling CI (backport micro-ROS#486) (micro-ROS#487)

* Fix Agent and host Rolling CI (micro-ROS#486)

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit d71ce51)

# Conflicts:
#	.github/workflows/ci.yml
#	.github/workflows/nightly.yml

* Fix 

Signed-off-by: Pablo Garrido <[email protected]>

Co-authored-by: Pablo Garrido <[email protected]>

* Fix CI upgrade ROS docker (micro-ROS#489)

Signed-off-by: Pablo Garrido <[email protected]>

* Remove Python 3.9 (backport micro-ROS#496) (micro-ROS#497)

* Remove Python 3.9 (micro-ROS#496)

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit d327cba)

# Conflicts:
#	.github/workflows/ci.yml

* Update

Signed-off-by: Pablo Garrido <[email protected]>

Co-authored-by: Pablo Garrido <[email protected]>

* Modified zephyr create.sh to pull correct toolchain based on arch (micro-ROS#499) (micro-ROS#501)

(cherry picked from commit 8793b5a)

Co-authored-by: Drew Hoener <[email protected]>

* Fix readme rosdep (micro-ROS#506) (micro-ROS#507)

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit c546b01)

Co-authored-by: Pablo Garrido <[email protected]>

* Add platformIO to Readme (micro-ROS#510) (micro-ROS#511)

(cherry picked from commit f0e7398)

Co-authored-by: Antonio Cuadros <[email protected]>

* Add renesas dummy meta (micro-ROS#516) (micro-ROS#517)

Signed-off-by: Pablo Garrido <[email protected]>
(cherry picked from commit c1ee153)

Co-authored-by: Pablo Garrido <[email protected]>

* Fix PyYaml unistall error (micro-ROS#521)

* Update changelog

Signed-off-by: Pablo Garrido <[email protected]>

* 1.1.0

* Remove unnessary check (micro-ROS#553) (micro-ROS#567)

Signed-off-by: Kevin Goez <[email protected]>

Signed-off-by: Kevin Goez <[email protected]>
(cherry picked from commit b81290a)

Co-authored-by: k7g03z <[email protected]>

Signed-off-by: Pablo Garrido <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Pablo Garrido <[email protected]>
Co-authored-by: Bhavesh Kakwani <[email protected]>
Co-authored-by: David Jablonski <[email protected]>
Co-authored-by: VictorLee <[email protected]>
Co-authored-by: Drew Hoener <[email protected]>
Co-authored-by: Antonio Cuadros <[email protected]>
Co-authored-by: k7g03z <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants