Sample: bluetooth: Enable MIMXRT1170 EVKB for peripheral_ht

[jerryyang35]

Enable MIMXRT1170 EVKB for peripheral_ht.

[github-actions]

Hello @jerryyang35, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

[lylezhu2012]

Hi @jhedberg , There is a building error from github action,

CMake Error at /__w/zephyr/zephyr/cmake/modules/extensions.cmake:1837 (message):
  Blob for path
  "/__w/zephyr/modules/hal/nxp/zephyr/blobs/iw612/uart_nw61x_se.h" missing.
  Update with: west blobs fetch
Call Stack (most recent call first):
  /__w/zephyr/zephyr/modules/hal_nxp/bt_controller/CMakeLists.txt:19 (zephyr_blobs_verify)

It seams the blob bin is not fetched by building system.

If we expect sample.bluetooth.peripheral-ht.nxp_nw612 to be compiled, is there any option that can instruct the building system to retrieve the blob?

Alternatively, we can only disable the building checking of sample.bluetooth.peripheral-ht.nxp_nw612. Is there an option to disable compilation?

[jhedberg]

@lylezhu2012 binary blobs are never fetched by upstream CI as a general policy. See the last paragraph of https://docs.zephyrproject.org/latest/contribute/bin_blobs.html#support-and-maintenance

Most likely you will need to deal with build testing in a downstream tree. One other possibility is the new CONFIG_BUILD_ONLY_NO_BLOBS option. Platforms can use that to provide empty stubs for the interface that the binary blobs provide. Both samples/bluetooth/peripheral and samples/bluetooth/central have test cases for platforms supporting this feature.

[lylezhu2012]

Most likely you will need to deal with build testing in a downstream tree. One other possibility is the new CONFIG_BUILD_ONLY_NO_BLOBS option. Platforms can use that to provide empty stubs for the interface that the binary blobs provide. Both samples/bluetooth/peripheral and samples/bluetooth/central have test cases for platforms supporting this feature.

I see. I found harness: bluetooth is added in sample.yml of samples/bluetooth/central. So in this PR, we can add harness: bluetooth, that the building will be ignored, right?

Sorry it is,

  sample.bluetooth.peripheral.no_blobs:
    extra_args:
      - CONFIG_BUILD_ONLY_NO_BLOBS=y

[jerryyang35]

Most likely you will need to deal with build testing in a downstream tree. One other possibility is the new CONFIG_BUILD_ONLY_NO_BLOBS option. Platforms can use that to provide empty stubs for the interface that the binary blobs provide. Both samples/bluetooth/peripheral and samples/bluetooth/central have test cases for platforms supporting this feature.

I see. I found harness: bluetooth is added in sample.yml of samples/bluetooth/central. So in this PR, we can add harness: bluetooth, that the building will be ignored, right?

Sorry it is,

  sample.bluetooth.peripheral.no_blobs:
    extra_args:
      - CONFIG_BUILD_ONLY_NO_BLOBS=y

it dose not work. the error message is the same.

[jerryyang35]

@lylezhu2012 binary blobs are never fetched by upstream CI as a general policy. See the last paragraph of https://docs.zephyrproject.org/latest/contribute/bin_blobs.html#support-and-maintenance

Most likely you will need to deal with build testing in a downstream tree. One other possibility is the new CONFIG_BUILD_ONLY_NO_BLOBS option. Platforms can use that to provide empty stubs for the interface that the binary blobs provide. Both samples/bluetooth/peripheral and samples/bluetooth/central have test cases for platforms supporting this feature.

I have added CONFIG_BUILD_ONLY_NO_BLOBS, but it dose not work. The error message is the same:
If I remove CONFIG_BT_NXP_NW612, there will be ' Unsupported controller.' error.

[jhedberg]

I have added CONFIG_BUILD_ONLY_NO_BLOBS, but it dose not work.

That's most likely because no-one ever added support for that option for NXP platforms. You need to do that first. For reference, you can see how I did this for the Silabs EFR32 HCI driver (which also depends on blobs) in commit 5f963fe.

[jerryyang35]

I have added CONFIG_BUILD_ONLY_NO_BLOBS, but it dose not work.

That's most likely because no-one ever added support for that option for NXP platforms. You need to do that first. For reference, you can see how I did this for the Silabs EFR32 HCI driver (which also depends on blobs) in commit 5f963fe.

Here I create a separated PR to support no blobs case for hal_nxp to avoid mixing these two tasks.
85848

[jerryyang35]

Hi @jhedberg .
After my careful consideration, I decided not to add the test in sample.yml, although zephyr CI builds are useful to ensure basic availability of our code change.
Zephyr do not support blobs in CI build. This leads to two problems for us:

1. Not good for CV test. Because CV always uses the image from CI, but the no-blobs image is not able to work.

2. Not good for users. Users must manually remove the no-blobs related config from sample.yml before debugging locally. And it is a bit complex for them to build the project when using sample.yml

[jhedberg]

@jerryyang35 ok, so you plan to close this then? Btw, I do think it'd be good to have all HCI drivers which may use blobs included in the "no_blobs" test cases defined as part of samples/bluetooth/central and samples/bluetooth/peripheral

[jerryyang35]

ok, so you plan to close this then? Btw, I do think it'd be good to have all HCI drivers which may use blobs included in the "no_blobs" test cases defined as part of samples/bluetooth/central and samples/bluetooth/peripheral

I will keep this PR. Now I use prj.conf to enable it.
Could you tell me why do you think it is good? From the build perspective, CI build is good. But CI cannot support blobs. So from the functionality perspective, I have the above two concerns.

[jhedberg]

Could you tell me why do you think it is good?

If you're referring to those peripheral & central test cases, the purpose for them is to validate the HCI driver code, i.e. that there are no build related bugs in them. I think that's better than no testing at all.

[jerryyang35]

Could you tell me why do you think it is good?

If you're referring to those peripheral & central test cases, the purpose for them is to validate the HCI driver code, i.e. that there are no build related bugs in them. I think that's better than no testing at all.

Hi @jhedberg.
We came up with a compromise that takes into account both CI and functionality.
For samples, we will take your advice, add our new board into sample.yml so that it can be test automatcally.
And we also make such a change in 86396, which ensure users can build a working version easily locally.

[jhedberg]

@jerryyang35 why are you specifically focusing on the peripheral_ht app? For BUILD_ONLY_NO_BLOBS I think it'd be good to consolidate testing that into the same apps for all blob-dependent drivers, and right now the two apps which have such test cases are peripheral and central. Do we really need to add peripheral_ht into the mix as well, and just for this single case?

[jerryyang35]

@jerryyang35 why are you specifically focusing on the peripheral_ht app? For BUILD_ONLY_NO_BLOBS I think it'd be good to consolidate testing that into the same apps for all blob-dependent drivers, and right now the two apps which have such test cases are peripheral and central. Do we really need to add peripheral_ht into the mix as well, and just for this single case?

Hi @jhedberg.
My purpose is to support bluetooth samples on 1170evkb which need an overlay to ensure the basic function. Can we just add the overlay without test case in sample.yaml?

[jhedberg]

Can we just add the overlay without test case in sample.yaml?

I don't know if there exists some general policy regarding this, but at least I'm ok with that approach.

[jerryyang35]

Can we just add the overlay without test case in sample.yaml?

I don't know if there exists some general policy regarding this, but at least I'm ok with that approach.

Thank you, @jhedberg. Do you know who is aware of general policy? May be we can get some suggestion from him/her.

[alwa-nordic]

@dleach02 or @mmahadevan108, can you check the overlay file for us? I don't know what zephyr,sram is, what its default is, and why this sample needs to specify it. It's not explained in the commit message or the file itself, but maybe it's obvious when you are familiar with the board?

[lylezhu2012]

@dleach02 or @mmahadevan108, can you check the overlay file for us? I don't know what zephyr,sram is, what its default is, and why this sample needs to specify it. It's not explained in the commit message or the file itself, but maybe it's obvious when you are familiar with the board?

I can give more comments. Due to the pin conflict between SDRAM and the M.2 module, all cases/samples will fail on the board where the M.2 module is installed.
Actually I have created a PR to set the default sram to dtcm. But it is closed due to the comemnts "I am blocking this for now, until NXP reviews the options. Using DTCM will limit the size of data memory, and will break some existing samples. If the M.2 module is instaled, the overlay can easily move data to DTCM."

Please refer to #70880 for details.