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

manifest: Give the option to get BabbleSim via the manifest #55696

Merged
merged 2 commits into from
Apr 20, 2023

Conversation

aescolar
Copy link
Member

@aescolar aescolar commented Mar 10, 2023

To improve users experience, give the option to users to fetch the required BabbleSim components using the west manifest.
Until now, users would need to either use the Zephyr docker image, or fetch bsim on their own.
For many years there has been a request to automate this process. This is the first step.

Fixes #55698

Note: A set of follow ups could/would:

  • For developers ease, have cmake find the simulator thru west if the environment variables that tell where
    the simulator is are not set. aescolar@9e331be
  • Add a cmake check to check the simulator is the necessary version, and otherwise instruct users to west update and rebuild it: aescolar@d9a49dc
  • Have CI use the version of the simulator in the manifest

@zephyrbot
Copy link
Collaborator

zephyrbot commented Mar 10, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
bsim N/A zephyrproject-rtos/babblesim-manifest@908ffde (main) N/A

Note: This message is automatically posted and updated by the Manifest GitHub Action.

west.yml Outdated Show resolved Hide resolved
@aescolar aescolar force-pushed the bsim_west branch 2 times, most recently from 091db62 to 74acb04 Compare March 11, 2023 14:09
@jori-nordic
Copy link
Collaborator

jori-nordic commented Mar 21, 2023

Built and ran the gatt settings test successfully with the bsim fetched using this PR.

@aescolar aescolar removed the DNM This PR should not be merged (Do Not Merge) label Mar 23, 2023
@stephanosio
Copy link
Member

@aescolar Regarding the following documentation build failure, you can cherry-pick 6c0b695 to fix the issue.

make: Entering directory '/home/runner/work/zephyr/zephyr/doc'
make html DT_TURBO_MODE=1
make[1]: Entering directory '/home/runner/work/zephyr/zephyr/doc'
cmake \
	-GNinja \
	-B_build \
	-S. \
	-DDOC_TAG=development \
	-DSPHINXOPTS="-q -W -t publish" \
	-DLATEXMKOPTS="-halt-on-error -no-shell-escape" \
	-DDT_TURBO_MODE=1
Loading Zephyr module(s) (Zephyr repository): doc
-- Found Python3: /usr/bin/python3.10 (found suitable exact version "3.10.6") found components: Interpreter 
-- Cache files will be written to: /home/runner/.cache/zephyr
-- Found west (found suitable version "1.0.0", minimum required is "1.0.0")
CMake Error at /home/runner/work/zephyr/zephyr/cmake/modules/zephyr_module.cmake:77 (message):
  Traceback (most recent call last):

    File "/home/runner/work/zephyr/zephyr/scripts/zephyr_module.py", line 718, in <module>
      main()
    File "/home/runner/work/zephyr/zephyr/scripts/zephyr_module.py", line 663, in main
      west_projs = west_projects()
    File "/home/runner/work/zephyr/zephyr/scripts/zephyr_module.py", line 531, in west_projects
      manifest = Manifest.from_file()
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 1170, in from_file
      return Manifest(topdir=topdir, config=config,
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 1396, in __init__
      self._load_validated()
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 1791, in _load_validated
      self._load_projects(manifest_data, url_bases, defaults)
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line [21](https://github.com/zephyrproject-rtos/zephyr/actions/runs/4502917193/jobs/7925415081?pr=55696#step:7:22)14, in _load_projects
      self._import_from_project(project, imp)
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 2300, in _import_from_project
      self._import_map_from_project(project, imp)
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 2327, in _import_map_from_project
      imported = self._import_content_from_project(project, imap.file)
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 2395, in _import_content_from_project
      content = self._ctx.project_importer(project, path)
    File "/home/runner/.local/lib/python3.10/site-packages/west/manifest.py", line 164, in _default_importer
      raise ManifestImportFailed(project, file)

  west.manifest.ManifestImportFailed: ManifestImportFailed: project <Project
  bsim ('/home/runner/work/zephyr/tools/bsim') at
  bc[22](https://github.com/zephyrproject-rtos/zephyr/actions/runs/4502917193/jobs/7925415081?pr=55696#step:7:23)891208d453a8cb851e4ecdc8f165f3a33890> value west.yml

Call Stack (most recent call first):
  /home/runner/work/zephyr/zephyr/cmake/modules/doc.cmake:24 (include)
  /home/runner/work/zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:75 (include)
  /home/runner/work/zephyr/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:117 (include_boilerplate)
  CMakeLists.txt:7 (find_package)


-- Configuring incomplete, errors occurred!
See also "/home/runner/work/zephyr/zephyr/doc/_build/CMakeFiles/CMakeOutput.log".
make[1]: *** [Makefile:[23](https://github.com/zephyrproject-rtos/zephyr/actions/runs/4502917193/jobs/7925415081?pr=55696#step:7:24): configure] Error 1
make[1]: Leaving directory '/home/runner/work/zephyr/zephyr/doc'
make: Leaving directory '/home/runner/work/zephyr/zephyr/doc'
make: *** [Makefile:17: html-fast] Error 2

@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label Mar 23, 2023
@aescolar aescolar marked this pull request as ready for review March 23, 2023 17:35
@aescolar aescolar removed the DNM This PR should not be merged (Do Not Merge) label Mar 23, 2023
west.yml Show resolved Hide resolved
@aescolar aescolar added the DNM This PR should not be merged (Do Not Merge) label Mar 24, 2023
@aescolar
Copy link
Member Author

Added DNM following @nashif request for further discussion

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good direction.

Let's take this one step further 😃

boards/posix/nrf52_bsim/find_babblesim.cmake Outdated Show resolved Hide resolved
aescolar and others added 2 commits March 30, 2023 16:50
To facilitate users experience, fetch the required
BabbleSim components using the west manifest.
Until now, users would need to either use the Zephyr
docker image, or fetch bsim on their own.
For many years there has been a request to automate this
process. This is the first step.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
This commit updates the documentation build workflow to pull in the
'bsim' west project (with its submanifest) so that the build process
does not fail due to a manifest import failure.

Signed-off-by: Stephanos Ioannidis <[email protected]>
Signed-off-by: Alberto Escolar Piedras <[email protected]>
@aescolar aescolar removed the DNM This PR should not be merged (Do Not Merge) label Apr 13, 2023
@stephanosio stephanosio requested a review from nashif April 20, 2023 09:01
@carlescufi carlescufi requested a review from gmarull April 20, 2023 09:43
@carlescufi carlescufi merged commit 00130b7 into zephyrproject-rtos:main Apr 20, 2023
@aescolar aescolar deleted the bsim_west branch April 20, 2023 13:48
@marc-hb
Copy link
Collaborator

marc-hb commented Apr 27, 2023

This breaks west update only what I need for all users:

aescolar added a commit to aescolar/zephyr that referenced this pull request Apr 18, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
nashif pushed a commit that referenced this pull request Apr 23, 2024
Babblesim has been in the manifest as an optional
set of west projects since
#55696
and
#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this pull request Apr 24, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos/zephyr#55696
and
zephyrproject-rtos/zephyr#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

(cherry picked from commit eddb382)

Original-Signed-off-by: Alberto Escolar Piedras <[email protected]>
GitOrigin-RevId: eddb382
Change-Id: Ieee5cae1c22fc7b1ec2303cec4150d6e34cc8f85
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5482306
Tested-by: ChromeOS Prod (Robot) <[email protected]>
Tested-by: Dawid Niedźwiecki <[email protected]>
Reviewed-by: Dawid Niedźwiecki <[email protected]>
Commit-Queue: Dawid Niedźwiecki <[email protected]>
jman168 pushed a commit to Gator-Motorsports/zephyr that referenced this pull request Apr 25, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
smaerup pushed a commit to smaerup/zephyr that referenced this pull request May 1, 2024
Babblesim has been in the manifest as an optional
set of west projects since
zephyrproject-rtos#55696
and
zephyrproject-rtos#59023

In principle we need one entry in the MAINTAINERS file
per west project, but as these project are optional,
entries for them were never added.
If a user has these babblesim project group enabled
locally, checkcompliance MaintainersFormat check will
warn though.
So let's fix this by adding the respective entries
in the MAINTAINERS file.

Signed-off-by: Alberto Escolar Piedras <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate bsim thru the west manifest