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

Make babbelsim testing more easily available outside of CI #20366

Closed
nashif opened this issue Nov 5, 2019 · 8 comments
Closed

Make babbelsim testing more easily available outside of CI #20366

nashif opened this issue Nov 5, 2019 · 8 comments
Assignees
Labels
area: Test Framework Issues related not to a particular test, but to the framework instead Enhancement Changes/Updates/Additions to existing features platform: nRF52 BSIM nrf52_bsim priority: medium Medium impact/importance bug
Milestone

Comments

@nashif
Copy link
Member

nashif commented Nov 5, 2019

Right now we do some special testing using babbelsim that is not available to users running sanitycheck locally. This can be disruptive when something fails in CI but can't be reproduced locally on the developer system. Add to that the fact that the setup of nrf52_bsim is not straightforward.

We should make this available through to anyone running sanitycheck and we should be able to whatever happens in CI on any machine where development happens.

We should look into pull the babbelsim code into the west manifest and make it available to sanitycheck in any local build and remove the special handling from the CI docker image.

@nashif nashif added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug labels Nov 5, 2019
@nashif nashif added the area: native port Host native arch port (native_posix) label Nov 5, 2019
@andrewboie
Copy link
Contributor

we should not be having special build configurations in CI like this.
I put a stop to doing this with ASAN, @nashif is there anything else lurking?

@aescolar
Copy link
Member

aescolar commented Nov 6, 2019

The way you present this is hardly fair, and certainly misleading. This was discussed very long ago, when this tests where added.

As discussed at that point:

  • The extra tests that are run outside of sanitycheck cannot be run by sanitycheck because they are multi-device tests. This part has nothing to do with BabbleSim, west or the nrf52_bsim. I can certainly elaborate more on this. But in short: To run one BT test with radio traffic, you need to build several images, and them run a simulation with all of them together and the physical layer simulation. That is why it uses a different framework. Everybody can use it locally if they have the dependencies, and many of us do.

  • Everybody can compile and run with the nrf52_bsim locally if they fetch the dependencies. But when it was added, it was your preference to simplify things for users (west did not even exist at the time mind you), so if people did not have what was needed present, the nrf52_bsim tests would be skipped silently, and so it was done.
    But if you have what you need to build the nrf52_bsim, it will run perfectly fine in sanitycheck.

  • The dependencies for the nrf52_bsim are BabbleSim (a tool, which has as much to do with Zephyr as cmake), and the nrf52 HW models. The HW models could be made a west module if we wanted (though they do not really fit specially well with the west module approach), but it still will require BabbleSim, which makes very little sense as a west module, just like cmake or ninja make very little sense as west modules.

I hope that you understand that this tools are needed in the BT development side. I can understand that those who do not care about BT, preferred to not need to bother spending any time fetching or being aware of it, just keeping it out of sight. But then, we ended up in the current situation. Where sometimes you will break something for the BT side, and you will be frustated.


I'm happy to improve things for users, and make it easier for them to fetch it and use it, but this is not a bug, and it is as it was agreed at that time. If anything this is an enhancement, maybe of high priority but an enhancement. And given that we are so close to 2.1 freeze, not something we can manage for 2.1

@aescolar aescolar added Enhancement Changes/Updates/Additions to existing features platform: nRF52 BSIM nrf52_bsim priority: high High impact/importance bug and removed area: native port Host native arch port (native_posix) bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug labels Nov 6, 2019
@aescolar aescolar changed the title Make babbelsim testing available outside of CI Make babbelsim testing more easily available outside of CI Nov 6, 2019
@aescolar aescolar added this to the v2.2.0 milestone Nov 6, 2019
@nashif
Copy link
Member Author

nashif commented Nov 6, 2019

The way you present this is hardly fair, and certainly misleading. This was discussed very long ago, when this tests where added.

what exactly is not fair?

I know this was discussed long ago, but we should reevaluate and reassess and make things simpler. I had to deal with all of this yesterday and it is certainly not easy to reproduce something that fails with sanitycheck/babblesim and in my case, I know the background and I know where to look.

Sure, we can make this an enhancement, I was not asking for this to be fixed in 2.1.

@stephanosio
Copy link
Member

@aescolar @galak Is there any specific reason why BabbleSim is not part of the Zephyr SDK?

If there isn't, I suppose we could add it to https://github.com/zephyrproject-rtos/sdk-ng so that it just works out of the box as long as you use the Zephyr SDK.

@aescolar
Copy link
Member

aescolar commented Nov 6, 2019

Is there any specific reason why BabbleSim is not part of the Zephyr SDK?

@stephanosio We could, there is 2 parts to this. One is the simulator basic components and 2.4GHz Phy which are very stable. So they can easily be part of the SDK (if people does not mind getting some more stuff).
Another is the nrf52 hw models, which are more unstable as they implement an nrfx hal replacement. (that's why the nrf52_bsim build has a check of the version of the HW models. And again, note that all this was done before west). The HW models could be made a west module, they could also be thrown into the the nrf52_bsim board folder, or so. But there is a small issue with license, as they inherit some BSD files from the nrfx HAL itself.

@aescolar
Copy link
Member

aescolar commented Nov 6, 2019

what exactly is not fair?

This:

Right now we do some special testing using babbelsim that is not available to users running sanitycheck locally. This can be disruptive when something fails in CI but can't be reproduced locally on the developer system.

we should not be having special build configurations in CI like this.
I put a stop to doing this with ASAN, @nashif is there anything else lurking?

It is available locally if you just install it. BabbleSim is open source (Apache2) and free. It is maintained and working. Last time I tried it, it could be fetched, compiled, and run just fine in RedHat 5 (meaning in an "ancient" distro)

Add to that the fact that the setup of nrf52_bsim is not straightforward.

Ok, let's then try to understand why it is not straight forward. Both you and @stephanosio have just installed. What hurdles did you find? Was the fetching and building documentation enough?

If just install it in, say /opt/ , and add the 2 env variables to bashrc (or zephyrrc) you are done, and everything works from there on. (Worst case, every few months you get a compile error with exact instructions on how to update the HW models)

@nashif
Copy link
Member Author

nashif commented Nov 6, 2019

It is available locally if you just install it. BabbleSim is open source (Apache2) and free. It is maintained and working. Last time I tried it, it could be fetched, compiled, and run just fine in RedHat 5 (meaning in an "ancient" distro)

Come on, this same statement can be said about anything that we have in the SDK for example, why do we bother putting things in the SDK? We could tell people to go build compilers and other tools following this model.

This might be easy to install for an experienced user 1 time, but how do you keep this part of local development without having to deal with various environment options and manual version checks and so on? This might work fine in CI, but if we are to do make this part of the development workflow, it needs to get easier and be better integrated with development tools such as sanitycheck and west etc, so as it is now, nobody should know about how to deal with this directly, it should just work.

@aescolar aescolar added priority: medium Medium impact/importance bug and removed priority: high High impact/importance bug labels Jan 3, 2020
@jhedberg jhedberg modified the milestones: v2.2.0, v2.3.0 Mar 10, 2020
@carlescufi carlescufi added the area: Test Framework Issues related not to a particular test, but to the framework instead label Apr 30, 2020
@carlescufi carlescufi modified the milestones: v2.3.0, v2.4.0 Jun 5, 2020
@nashif
Copy link
Member Author

nashif commented May 28, 2021

I think this is now available to everyone using the dokcker image which can be run and used by anyone.

@nashif nashif closed this as completed May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Test Framework Issues related not to a particular test, but to the framework instead Enhancement Changes/Updates/Additions to existing features platform: nRF52 BSIM nrf52_bsim priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

6 participants