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

Integrate how to compile documentation to the codebase #287

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

samuelbles07
Copy link
Collaborator

@samuelbles07 samuelbles07 commented Feb 24, 2025

Relate to this issue: #160


![Compile Settings](images/settings.png)

4. Compile
Copy link
Contributor

Choose a reason for hiding this comment

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

A large missing piece of this howto is how to get all of the required files from Github.

To try to see things as a new user, I followed these instructions in a new Windows install. When I got to this step about compiling, it didn't say which file to compile, so I grabbed OneOpenAir.ino from the repo, saved it to my device in Documents/Arduino folder, and it fails with an error about LocalServer.h not available.
image

I downloaded the entire repo as a .zip file and then extracted arduino-master to Documents/Arduino folder. Then opened the file in the IDE C:\Users\WDAGUtilityAccount\Documents\Arduino\arduino-master\examples\OneOpenAir\OneOpenAir.ino

This let me compile the firmware exactly as it is, but still relies on the released of the AirGradient library to the Arduino Library manager. If the reason a user is wanting to compile their own is so they can modify the code, their local IDE needs to be using the local files entirely and not using the published Library. If I uninstall "AirGradient Air Quality Sensor" from the library manager, I can no longer compile the firmware, as there are files that are not being used by the IDE
image

For me, the only way to do this is to add the repository files to be a subfolder of Arduino/Libraries, so the IDE is using my local files as part of the library so it can find them appropriately. So instead of putting the contents of the Github repo into Documents/Arduino, I put them in Documents/Arduino/libraries and opened the .ino file and then I was able to compile successfully with all local code, letting me make modifications and have them be used when compiling.
C:\Users\WDAGUtilityAccount\Documents\Arduino\libraries\arduino-master\examples\OneOpenAir\OneOpenAir.ino

So I think all of this needs to be covered somewhere and somehow

Copy link
Owner

Choose a reason for hiding this comment

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

@MallocArray thanks for the feedback. Achim here.

When I played around with the Arduino IDE, I saw quite a few restrictions in file locations etc. to get things compiled. I wonder if we can make the assumption that if people use the Arduino IDE they will use the AirGradient Arduino library and install it with the library manager?

If people use other IDE's then everything should work with cloning the repo etc.

Not sure if that makes sense. Just want to throw this into the discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that is a safe assumption. Issue #160 is all about people not being able to compile from scratch and I think the safe assumption on why people would want to compile it themselves instead of using the Firmware installer would be either to audit the software before using it, or to make changes to it, and I think making changes would be the more likely chance.

If the instructions provided to compile don't actually use the files that the user is changing locally, I think that is a major disservice to the user and potentially a big black eye to the AirGradient project. You are proud of being open source, but if end users can't use the source code, or are frustrated by attempting to use what is provided, I don't think that is a good look for AirGradient.

I am not an Arduino expert by any means, but I am an engineer and I use several languages and IDEs. I didn't even know I had other options outside of the Arduino IDE. But if the compiling instructions say to use the Arduino IDE, I think it should be a well rounded document to help users be able to compile their own code successfully, and missing the steps about how to use the local version of the files is going to cause extra confusion and disatisfaction.

Copy link
Owner

Choose a reason for hiding this comment

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

I agree. I just wanted to point out that the Arduino editor has some strange design choices that are a bit unusual but I'm also not deep enough in that topic anymore. I talked to the team and we will try to address these issues as good as we can.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just add another step on how to choose the sketch based on models, this should solve the confusion for not manually download file from the repository.

Copy link
Contributor

@MallocArray MallocArray Feb 27, 2025

Choose a reason for hiding this comment

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

I just add another step on how to choose the sketch based on models, this should solve the confusion for not manually download file from the repository.

This is an improvement as it does get it to compile.
One thing that is not clear is where the files are stored after you install the AirGradient library, as it kind of "just works" but at least in the IDE itself I don't see where it indicates where the sketch is located.

Since an expected use case is users wanting to make their own modifications which could mean needing to modify files in the src folder, at least a mention that the Library files are located by default in the Documents/Arduino/libraries for Windows and Mac, and ~/Arduino/libraries for Linux would be helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this file is referenced by the howto-compile.md document any longer and can be removed


![Compile Settings](images/settings.png)

4. Compile
Copy link
Contributor

@MallocArray MallocArray Feb 27, 2025

Choose a reason for hiding this comment

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

I just add another step on how to choose the sketch based on models, this should solve the confusion for not manually download file from the repository.

This is an improvement as it does get it to compile.
One thing that is not clear is where the files are stored after you install the AirGradient library, as it kind of "just works" but at least in the IDE itself I don't see where it indicates where the sketch is located.

Since an expected use case is users wanting to make their own modifications which could mean needing to modify files in the src folder, at least a mention that the Library files are located by default in the Documents/Arduino/libraries for Windows and Mac, and ~/Arduino/libraries for Linux would be helpful.

Comment on lines +67 to +71
ModuleNotFoundError: No module named ‘serial’

Make sure python pyserial module installed globally in the environment by executing:

`$ pip install pyserial`

Choose a reason for hiding this comment

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

I appreciate this section, however people are going to run into problems installing the library this way. This should be sudo apt install -y python3-pyserial instead.

Here's what happens if I install sudo apt install -y python3-pip (it's not installed by default) and then run the command above:

$ pip install pyserial
error: externally-managed-environment

× This environment is externally managed
╰─> To install Python packages system-wide, try apt install
    python3-xyz, where xyz is the package you are trying to
    install.
    
    If you wish to install a non-Debian-packaged Python package,
    create a virtual environment using python3 -m venv path/to/venv.
    Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
    sure you have python3-full installed.
    
    If you wish to install a non-Debian packaged Python application,
    it may be easiest to use pipx install xyz, which will manage a
    virtual environment for you. Make sure you have pipx installed.
    
    See /usr/share/doc/python3.11/README.venv for more information.

note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
hint: See PEP 668 for the detailed specification.

pip does have a --break-system-packages option, but as you can imagine, this is generally not a good idea. Making a virtual environment as the error message suggests is one option, but a much simpler option is to use apt to install the package.

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.

5 participants