Skip to content

Conversation

@joepardue
Copy link
Contributor

This pull request adds my CircuitPython driver for the AMS AS7343 spectral sensor.

  • Submodule added under libraries/drivers/circuitpython-as7343
  • Entry added to circuitpython_community_library_list.md

Library repository: https://github.com/joepardue/circuitpython-as7343

Thanks for reviewing!

joepardue added 3 commits May 8, 2025 12:01
Novice trying to get my bundle shared.
Add .gitmodules entry for circuitpython-as7343
Add circuitpython-as7343 to drivers list
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this new driver. I think we need a few more things before it's ready to merge.

@@ -0,0 +1 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this part of the git add didn't work properly or something. It's showing just an empty .gitmodules file as the contents of the new repo. Ordinarily in the github interface this would show a link out to the other repo and a hash of the current commit:
image

I'm not sure of the exact right way to fix it. I suspect it will be something like deleting this empty .gitmodules file and then updating submodules in this branch that has the new library added, that should fetch the contents of the new library which could then be commit / pushed to this branch I think.

I'll try to tinker with this a little later and see if I can figure out the way to get it corrected with more certainty.

@joepardue
Copy link
Contributor Author

joepardue commented May 8, 2025 via email

@joepardue
Copy link
Contributor Author

I went down a two day rabbit hole that generated 65 failed builds. I'm going to back up and carefully restart after rereading the docs. This can't be this hard!

@joepardue
Copy link
Contributor Author

I have struggled with this. The name is not in the format normally used, but I killed the whole thing several times trying to fix that. Also, the README files are not what I want, I have much more detailed ones, but I want to see if this thing will actually go through now. I had 65 Build Cl rejections last time so I am totally fried and fearful of what will happen now. If this passes, then I'll try to update the README and if that works, I'll try to get the name in conformity with the rest of the library files.

@FoamyGuy
Copy link
Contributor

Thanks for working through the actions checks! This is very close to ready.

We don't ordinarily want 3rd party community libraries to include "adafruit" in the name which this package does inside of your repo. I've submitted a PR over on your repo: joepardue/circuitpython-as7343#1 to rename the package.

I think if you merge that PR (or make your own commits with the same changes) and then make a new release for your library this should be good to go.

@joepardue
Copy link
Contributor Author

I've fixed the build issue that was reported:

  1. The syntax error in pyproject.toml has been corrected (there was a missing newline after [build-system])
  2. Created a new release (v1.0.3) with the fix
  3. Updated the submodule in my fork to point to the latest release

The library should now build correctly. Please let me know if there are any other issues that need to be addressed.

@FoamyGuy
Copy link
Contributor

@joepardue thanks again for working through those issues.

One more thing I just noticed is that there are extra nested directores with a leftover empty gitmodules file in it at:

libraries/drivers/libraries/drivers/circuitpython-as7343/.gitmodule

I think you should be able to fix that by running this command inside your CircuitPython_Community_Bundle repo:

git rm -r libraries/drivers/libraries/drivers/circuitpython-as7343/

after that make a commit and push it to this branch and I think it should be fixed.

When you make PRs there should be an "allow maintainers to add commits" option or something similar that if you have allowed it can let the project maintainers add commits to fix things like this if you are comfortable with that. I tried to commit this fix but it was not allowing me.

@FoamyGuy
Copy link
Contributor

@joepardue
I think the versions issue that the actions failure has in the log is due to the way the releases on the library are:
image

The newest released 1.0.3 isn't tagged as the "latest" release for github. I think if you go to the release and click edit near the top right there should be a checkbox or button on the edit page to mark it as the "latest" release. The bundle build tool will always look for the one tagged "latest" so it's good to have that always as the newest one.

I believe the version number on the prior release is part of the issue as well 1.02 I believe the build tool expects that the release version tag will always be a 3 digit semantic version value so it must have two dots, 1.0.2 would be valid, but 1.02 is not.

@joepardue
Copy link
Contributor Author

FoamyGuy, I think I did some work while you were commenting, so I'm not sure where we are at the moment. I submitted a more comprehensive README file and broke something, so let me look into that. I really do appreciate your looking at this. I'm slowly figuring it out.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Aweomse! Thank you @joepardue

@FoamyGuy FoamyGuy merged commit a2e1a78 into adafruit:main May 16, 2025
1 check passed
@joepardue
Copy link
Contributor Author

joepardue commented May 16, 2025 via email

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