Skip to content

wifi: winc1500: Update HAL version to 19.7.6 #51

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lromeraj
Copy link

@lromeraj lromeraj commented Aug 13, 2025

Rename the configuration symbol in nm_bsp_internal.h from CONFIG_WIFI_WINC1500 to CONFIG_ATMEL_WINC1500 to decouple the BSP header inclusion from the default Zephyr driver.

This change allows alternative driver implementations to use the BSP headers without being forced to enable CONFIG_WIFI_WINC1500, which otherwise enables the built-in Zephyr driver and can cause conflicts when providing a custom driver.

@lromeraj lromeraj changed the title wifi: winc1500: Rename CONFIG_WIFI_WINC1500 to CONFIG_ATMEL_WINC1500 … wifi: winc1500: Rename CONFIG_WIFI_WINC1500 to CONFIG_ATMEL_WINC1500 Aug 13, 2025
@nandojve nandojve self-assigned this Aug 13, 2025
@nandojve
Copy link
Member

Hi @lromeraj ,

Change is fine for me but you need to follow PR rules here as in Zephyr. See README in this repo.

@lromeraj
Copy link
Author

Hi @nandojve,

Thanks! I will also create the corresponding PR in the Zephyr repository.

I’m currently updating the HAL (aka host_drv) to version 19.7.6, which I downloaded here. This version introduces new features and fixes, and it requires some modifications on the Zephyr side as well—but I already have a working version.

I understand that a PR in the Zephyr repository is needed too. Do you think it makes sense to include the HAL update in this PR, or would you prefer a separate PR for the new HAL version?

@nandojve
Copy link
Member

@lromeraj ,

Do you think it makes sense to include the HAL update in this PR, or would you prefer a separate PR for the new HAL version?

Yes, I particularly never saw the module working and I would like to have it. Then open the correspondent PR in Zephyr with the bump + changes. Pay attention to no break bisect in Zephyr repo. So, if the bump requires at same time the changes in Zephyr you need to make sure add in the same commit. We care about bisect that allow us to build and don't break other platforms.

@lromeraj
Copy link
Author

lromeraj commented Aug 18, 2025

Hi @nandojve,

I’ll continue working on this, but I’ll need a few days to fully develop and test the new implementation. I’ve opened the PR in the Zephyr repository as a draft. I’ll keep my branch up to date by force-pushing to the same commit.

@lromeraj lromeraj marked this pull request as draft August 18, 2025 13:31
@lromeraj lromeraj changed the title wifi: winc1500: Rename CONFIG_WIFI_WINC1500 to CONFIG_ATMEL_WINC1500 wifi: winc1500: Update HAL version to 19.7.6 Aug 18, 2025
@nandojve
Copy link
Member

Hi @lromeraj ,

No worries, take your time!
I saw and add myself in the list because then I can easy find the PR.
When do you believe everything is fine you simple switch to ready

@lromeraj
Copy link
Author

lromeraj commented Aug 19, 2025

Hi @nandojve,

I’d like to change the source branch because I realized I created the pull request from master, which isn’t ideal. I can open a new PR from the correct branch. What do you think? My current work is in the following branch

@nandojve
Copy link
Member

I don't think this is a problem because it is your main branch.
I think you can keep it but you need to be more careful in the future.

@lromeraj
Copy link
Author

@nandojve Sorry, I completely missed that. I’ll push the changes to the master branch instead.

@lromeraj
Copy link
Author

@nandojve I’d like to propose an idea: since the WINC1500 supports OTA updates, it would be useful to upload the corresponding binary to a location in this repository. This way, we could set a default URL in the Kconfig pointing to the binary, making the download and update process straightforward.

What do you think? Perhaps we could use GitHub Pages or another method to make the file publicly accessible.

@nandojve
Copy link
Member

Hi @lromeraj ,

There is a specific rules about blobs in the docs. I'll ask you to read those sections.
Let me know if something else is missing.

@lromeraj
Copy link
Author

@nandojve I see. Uploading the binary elsewhere doesn’t feel safe to me. I’d prefer keeping it in the repository so we have full control over modifications and updates.
Another option could be a West extension command to download the resource with extra steps (unpack, check, serve…), but that seems more complex than simply hosting the blob in the repo’s wiki or pages.
If that’s not possible, I’d rather not proceed with this functionality.

@lromeraj
Copy link
Author

lromeraj commented Aug 20, 2025

@nandojve Regarding the required HAL modifications: I’m using the __ZEPHYR__ macro to detect when we build for Zephyr. I usually follow this approach in Zephyr modules for two main reasons:

  • It makes HAL modifications easier to find.
  • It allows the library to be built without modifications if needed (e.g., a future host CLI provided by this module might require an unmodified HAL).

Several core socket functions (e.g., accept(), listen(), etc.) were renamed in this repository. For this case, there are multiple ways to handle renaming, but I chose to define macros in the header file. At the end of the header, I #undef them to avoid potential conflicts in the driver implementation that relies on socket.h.

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.

2 participants