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

Added compilation steps #5117

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

David-OConnor
Copy link
Contributor

@David-OConnor David-OConnor commented May 9, 2023

This adds explicit compilation steps to the add-build-steps documentation. Should make things easier on new users, and the heros who have been helping them on Discord and GH.


- Linux and MacOSX users should build with waf as described in `BUILD.md <https://github.com/ArduPilot/ardupilot/blob/master/BUILD.md>`__.
- `git submodule init`
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is required to avoid the following error message:

Build failed
 -> task in 'dronecan' failed (exit status 2):
        {task 140083940872752: dronecangen  -> }
 (run with -v to display more information)

- Linux and MacOSX users should build with waf as described in `BUILD.md <https://github.com/ArduPilot/ardupilot/blob/master/BUILD.md>`__.
- `git submodule init`
- `git submodule update --recursive`
- `./Tools/gittools/submodule-sync.sh`
Copy link
Contributor

Choose a reason for hiding this comment

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

this should only be used in case of emergency....its a sledge hammer....please remove it....we can add a note in the branching doc about using it in case of emergency where you cant get the modules updated correctly

Copy link
Contributor Author

@David-OConnor David-OConnor May 10, 2023

Choose a reason for hiding this comment

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

submodule init line removed It seems the other two are required for me. The first line prevents this error:

Build failed -> task in 'dronecan' failed (exit status 2): {task 140083940872752: dronecangen -> } (run with -v to display more information)

The second prevents this:

Build failed
 -> task in 'ChibiOS_lib' failed (exit status 2):
        {task 140009737052320: ChibiOS_lib hwdef.h,hw.dat,include_dirs,board.c,board.h,bouncebuffer.c,bouncebuffer.h,chconf.h,crashdump.c,ffconf.h,flash.c,flash.h,halconf.h,hrt.c,hrt.h,malloc.c,mcuconf.h,poll.h,ppm.h,spi_hook
        // ... (many lines of red text)

Copy link
Contributor

Choose a reason for hiding this comment

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

not needed on fresh build

@David-OConnor
Copy link
Contributor Author

Thank you for the review: After some experimenting based on your fixes, I committed most of your changes. These two items though, seem to be required:

- `git submodule update --recursive --init`
- `./Tools/gittools/submodule-sync.sh`

@Ryanf55
Copy link
Contributor

Ryanf55 commented May 10, 2023

Thank you for the review: After some experimenting based on your fixes, I committed most of your changes. These two items though, seem to be required:

- `git submodule update --recursive --init`
- `./Tools/gittools/submodule-sync.sh`

After running the git submodule update --recursive --init, can you share the output of
git submodule status and git status?

@Hwurzburg
Copy link
Contributor

Thank you for the review: After some experimenting based on your fixes, I committed most of your changes. These two items though, seem to be required:

- `git submodule update --recursive --init`
- `./Tools/gittools/submodule-sync.sh`

only the update should be necessary....if the modules are re-organized (not changed) since you last updated, then the submodule-sync would be required AFAIK...this is a very rare occurrence and is usually announced

@David-OConnor
Copy link
Contributor Author

Thank you for the review: After some experimenting based on your fixes, I committed most of your changes. These two items though, seem to be required:

- `git submodule update --recursive --init`
- `./Tools/gittools/submodule-sync.sh`

After running the git submodule update --recursive --init, can you share the output of git submodule status and git status?

david@Data-Desktop:~/ardupilot$ git submodule update --recursive --init
david@Data-Desktop:~/ardupilot$ git submodule status
 2696de2e47f8d83a3e4d24c4942cc4a8f5342cee modules/ChibiOS (heads/master)
 599965086437137ec0fe66e185611f43f335f889 modules/CrashDebug (heads/master)
 94ea1e98ffbb9d24b6717905e092dba2f211d1a1 modules/DroneCAN/DSDL (94ea1e9)
 ebaf96860a11a4cc43c01df6b651df143c6cde2d modules/DroneCAN/dronecan_dsdlc (heads/master)
 f9c584fe844af71a840d9b8d915b7a448c394ff6 modules/DroneCAN/libcanard (f9c584f)
 19fdf2e5b383243ccdb1094edae0603cf11469e8 modules/DroneCAN/pydronecan (1.0.16-2-g19fdf2e)
 3d1b17703c7cf4f22def2910bc845bdb5152d7b5 modules/Micro-CDR (heads/master)
 a2937bc5af606b2434e9a841adbf5c003fc1ec38 modules/Micro-XRCE-DDS-Client (heads/master)
 d572f4777349d43653b21d6c2fc63020ab326db2 modules/gbenchmark (v1.7.1)
 e1f690585d4803402584962bfaa8240ecaf1db30 modules/gsoap (heads/master)
 c5fed93f941865a0e912e9baf46ded713506590a modules/gtest (release-1.8.0-2268-gc5fed93f)
 9c39822c0f6bef4e7ecb5c4f95979f06c23f763d modules/mavlink (1.0.12-331-g9c39822c)
 1b1625b8e7da6e1307d73335cb995fa8813d5950 modules/waf (waf-1.9.2-731-g1b1625b8)
david@Data-Desktop:~/ardupilot$
david@Data-Desktop:~/ardupilot$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   libraries/AP_HAL_ChibiOS/hwdef/MatekH743/hwdef-bl.dat
        modified:   libraries/AP_HAL_ChibiOS/hwdef/MatekH743/hwdef.dat

no changes added to commit (use "git add" and/or "git commit -a")
david@Data-Desktop:~/ardupilot$


- Linux and MacOSX users should build with waf as described in `BUILD.md <https://github.com/ArduPilot/ardupilot/blob/master/BUILD.md>`__.
- `git clone https://github.com/ArduPilot/ardupilot`
Copy link
Contributor

Choose a reason for hiding this comment

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

we have setup the enviroement in previous setup, so no need to git clone again

- Linux and MacOSX users should build with waf as described in `BUILD.md <https://github.com/ArduPilot/ardupilot/blob/master/BUILD.md>`__.
- `git clone https://github.com/ArduPilot/ardupilot`
- `cd ardupilot`
- `git submodule update --recursive --init`
Copy link
Contributor

Choose a reason for hiding this comment

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

we shouldn't need this as previous instruction have done it already

Copy link
Contributor

Choose a reason for hiding this comment

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

A link to those instructions here would suffice.

Copy link
Member

@hendjoshsr71 hendjoshsr71 Oct 6, 2023

Choose a reason for hiding this comment

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

I disagree.

Somewhere on the welcome to dev with ardupilot we must show how to use the basics of waf.

That little link to the giant list in build.md is easy to miss and has too much detail...
We need a cohesive easy to follow story.

I originally figured out how to use waf because I watched the commands the ArduPilot vscode extension was producing.

- Linux and MacOSX users should build with waf as described in `BUILD.md <https://github.com/ArduPilot/ardupilot/blob/master/BUILD.md>`__.
- `git submodule init`
- `git submodule update --recursive`
- `./Tools/gittools/submodule-sync.sh`
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed on fresh build


.. youtube:: lNSvAPZOM_o
`*BoardName*` above is the name of the board, as labeled by its associated folder in
`/ardupiot/libraries/AP_HAL_ChibiOS/hwdef`.
Copy link
Contributor

Choose a reason for hiding this comment

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

no, board name isn't restricted to Chibios boards.
./waf list_boards show all the boards

@Hwurzburg
Copy link
Contributor

I gave this another look....carefully walking thru the existing wiki path for setting the build environment and then building the code, I am afraid this PR does not add anything but duplication.....I am willing to hear an argument against that...but am leaning on closing this as unnecessary duplicaton...

it does highlight the need to perhaps expand the "contributing code" section on the use of submodule updates and waf clean and the submodules-sync script....since those only are needed (assuming one follows the wiki in setup and building) if you create or rebase local branches for code contribution and modules have gotten out of date, etc....but not just building from a properly cloned master, which the wiki leads you thru in the "Building the code" section

@David-OConnor
Copy link
Contributor Author

David-OConnor commented May 11, 2023

Hi. Going through as a new user, a list of commands needed to compile is important. Let's walk through the build steps on Windows using this page:

  • Link to Setup the Build Environment on Windows: This instructs the user to install WSL, or Cygwin.
  • "Windows users: Windows users should fellow the directions described on Setup Build Environment on WIndows"

Do you see the problem? There are no Windows build instructions here! It's an infinite loop. If you compare it to the page post this PR, a clear set of instructions are provided for all OSes. Ie, this PR makes it clear you need to build from a Linux environment, and tells the user to explicitly follow the Linux instructions.

Additionally, for the Linux page in BUILD.md, there are many steps. It's quite complicated. This PR adds a streamlined instructions, without navigating to a different page, of how to build. In my opinion, the page altered by the instructions clearer for Windows and Linux users.

BUILD.md as it stands is a great reference, but is not a good summary of required steps.

I currently maintain a text document with the build recipe; I shouldn't have to do this. This PR is my attempt to share the knowledge in an explicit way, so the next person doesn't need similar personal notes. If you'd like to reject it, that's fine, but consider if the instructions are better before, or after this PR.

Here's another way we can handle this: Close this PR, and make a new one that solves the same issues in a way you'd like. This might save some back+forths on wording etc. I think I've contributed all I can here content wise and raising why the docs as-is weren't sufficient, and why I had to bother people on Discord.

Thank you in particular to @peterbarker, @hendjoshsr71, and @MichelleRos ! My intent here is to reduce the use of people's personal time. I had trouble. I tried troubleshooting myself and got nowhere. These people helped me out a lot on discord. Rather than keep it to myself, I'd like others to benefit.

Here's a link to the PRed file directly, as the Files changed link here is a bit tough to parse with the comments:

https://github.com/David-OConnor/ardupilot_wiki/blob/add-build-steps/dev/source/docs/building-the-code.rst

Original: https://ardupilot.org/dev/docs/building-the-code.html

@MichelleRos
Copy link
Contributor

MichelleRos commented May 11, 2023

@David-OConnor

Do you see the problem? There are no Windows build instructions here!

I think there's two different things here - setting up the build environment, and compiling.

For the setup, this is actually correct... ArduPilot can't be built on Windows at all, which is why step one to build ArduPilot on Windows is essentially "install some form of Linux VM".

For compiling, the next step after the Setup the Build Environment on Windows page you mentioned actually explains how to setup VS Code to have "tasks" to build for various boards. I just took the alternative method, which is to just use the Ubuntu commands since WSL is essentially just an Ubuntu VM. I would just add a note to that page mentioning the VS Code setup (https://ardupilot.org/dev/docs/building-setup-windows.html) saying people could alternatively just follow the Ubuntu instructions if they prefer command-line.

@David-OConnor
Copy link
Contributor Author

David-OConnor commented May 11, 2023

The build instructions for windows are build in an emulated Ubuntu. To an experienced developer, this is clear. To a new person, who the docs are for, this should be spelled out. IDE choice is orthogonal; there is a recipe for building from Windows, and it's what's in this PR.

As it stands, here's the instructions for windows users, unwrapped:

Consider if these steps make sense or are easy to follow for someone who's never developed with AP. And consider if, back to back, the pre-this-PR and post-this-PR building page is easier to follow.

On the other note re the lines we've been going back+forth on (to prevent the errors I listed above) , why do y'all suppose we're seeing different things re it working without them or not? I've done a lot of testing over the past few days with several fresh environments, and they're required each time.

@MichelleRos
Copy link
Contributor

MichelleRos commented Aug 28, 2023

It tells you how to set up WSL and install Ubuntu within it, yes, but then at step 3 on that page it says

From the Start menu, start the “Ubuntu” application and then follow the Ubuntu instructions to install ArduPilot development environment

which is how you then setup & build ArduPilot within that Ubuntu.

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.

7 participants