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

[DNM] modules: Add the Arduino Zephyr API #72993

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DhruvaG2000
Copy link
Contributor

Add the Arduino Core API for zephyr as an optional module

@zephyrbot
Copy link
Collaborator

zephyrbot commented May 19, 2024

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

Name Old Revision New Revision Diff
gsoc-2022-arduino-core N/A zephyrproject-rtos/gsoc-2022-arduino-core@9e6adb1 (next) N/A

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

@zephyrbot zephyrbot added manifest manifest-gsoc-2022-arduino-core DNM This PR should not be merged (Do Not Merge) labels May 19, 2024
@DhruvaG2000 DhruvaG2000 marked this pull request as ready for review May 19, 2024 14:11
Add the Arduino Core API for zephyr as an optional module
However, don't make the group as optional because it will
make it an "inactive project" and so get ignored by west.
This will in turn fail to recognise cmakelists and kconfig
from the module leading to build failures.

Signed-off-by: Dhruva Gole <d-gole@ti.com>
@DhruvaG2000
Copy link
Contributor Author

I don't know why it's not assigning any reviewers, hence tagging manually
CC @nashif @carlescufi @soburi

Copy link
Member

@Ayush1325 Ayush1325 left a comment

Choose a reason for hiding this comment

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

Hi, I think there needs to be a maintainer entry for this module.

@soburi
Copy link
Member

soburi commented May 20, 2024

@DhruvaG2000

First of all, I think it's too early since there is no SPI implementation yet.
SPI should be included when writing a device overlay, but I'm not in a position to test it.
(Most of all, most people who use it will think SPI works!)
This should be implemented soon.

Also, as I previously commented, I think there will be a need to discuss the GPL license. I think, ultimately, it will need to be agreed upon by TSC, but the PR needs to include the information necessary to make the problem known.

@DhruvaG2000
Copy link
Contributor Author

Hi @soburi ,

SPI should be included when writing a device overlay,

Yes, I agree to those points, but as you can see in this PR I am not adding any code as such to upstream. This is just an effort to ensure that people are able to seamlessly pull in the arduino core module after they've cloned zephyr without having to dig around much or needing to make manual changes to the main yml file just to pull in the arduino module.

I have referred to the Documentation here: https://docs.zephyrproject.org/latest/contribute/external.html#integration-as-optional-modules

It states:

Optional projects that provide value to users directly and through a Zephyr subsystem or platform shall be added to an optional manifest file that is filtered by default. (submanifests/optional.yml).

It was my understanding that the Arduino core API is one such optional project directly under zephyr and users of zephyr rtos should be made aware within optional.yml if atall they're interested to pull it in and use it (Ofcourse subject to limitations of whatever the project supports at that point of time)

The other option is also this: https://docs.zephyrproject.org/latest/contribute/external.html#integration-as-external-modules

If "as external modules" sounds better to everyone I am open to that as well, let me know. It is to be noted that

I think there will be a need to discuss the GPL license

This too has been a long standing issue, and I get the part that integrating code in a single repo can be an issue, but in this PR I plan on bringing no such code directly inside zephyr-rtos anywhere. This is still completely external as a lib/module.

What I should probably do now is create the issue as told in the documentation https://github.com/zephyrproject-rtos/zephyr/issues/new?assignees=&labels=RFC&template=007_ext-source.md&title= to get reviewed by the Technical Steering Committee (TSC) .

@DhruvaG2000 DhruvaG2000 changed the title modules: Add the Arduino Zephyr API [DNM] modules: Add the Arduino Zephyr API May 20, 2024
@DhruvaG2000
Copy link
Contributor Author

Hi, I think there needs to be a maintainer entry for this module.

Good catch, I think I am also missing the changes to be made in modules/ like Kconfig and CMakelists atleast (if I were to look at for eg. https://github.com/zephyrproject-rtos/zephyr/tree/main/modules/lz4 )
But I will let the TSC comment on the issue first and see if they're okay with the idea in the first place.

@carlescufi
Copy link
Member

@DhruvaG2000 please follow the instructions here:

https://docs.zephyrproject.org/latest/contribute/external.html

although in this case it may be considered an "internal" project, but it still needs discussion in the TSC

@carlescufi carlescufi added the TSC Topics that need TSC discussion label May 20, 2024
@soburi
Copy link
Member

soburi commented May 21, 2024

@DhruvaG2000 @carlescufi

The premise of my opinion is that if we are going to merge features, they need to be easily available and useful enough for general users.
I believe that there are still issues to be resolved at present.
Me and @DhruvaG2000 may have different opinions on this point.

It is true that the module is not part of the GPL, but the method of "downloading GPL part at the user's responsibility" is incomplete, and it is necessary to think about how to properly incorporate it into the module.
(In other words, We need to seriously think about how to handle GPL within your zephyr structure.)

Dialogue with TSC is necessary to resolve this issue, and it is best to start this dialogue early. I think it's great that you created this issue.

I believe there are two issues that need to be resolved as soon as possible. I think these issues need to be discussed in parallel with licensing issues for the merge.

  1. Developing user usage methods
    It is necessary to be able to easily use this function from west command.
    (As an example of integration, I considered a method using the shield structure. Currently, snippet can be used, so I think it is preferable to use it. Integrate arduino api #64051)

  2. Implementation of required APIs (As I mention in above)
    Digital (GPIO), Analog (ADC), PWM, and I2C are currently being implemented. Since SPI requires changes to dts overlay, I think it need to implement this before release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM This PR should not be merged (Do Not Merge) manifest manifest-gsoc-2022-arduino-core TSC Topics that need TSC discussion
Projects
Status: Under Review
Development

Successfully merging this pull request may close these issues.

None yet

5 participants