Skip to content

Latest commit

 

History

History
617 lines (468 loc) · 23.5 KB

CONTRIBUTING.md

File metadata and controls

617 lines (468 loc) · 23.5 KB
Raw

How to contribute to esp-idf-lib

Table of Contents

Possible contributions

If you would like to contribute to esp-idf-lib, we would like to say thank you. We appreciate your efforts and contributions. Here is possible contributions you can make.

Submitting a bug report

In embedded device development, finding bugs is more difficult than in other software development. There are too many uncontrollable factors: physical environment, counterfeit IC chips, deviations in revisions and variations, difficulties in automations. Even if the bug turned out to be not a bug, such report is still appreciated as it is another proof that the code works as expected in a different environment.

Please include how to reproduce the bug in the Issue. The more context, the better. For example:

  • The full error message in text format and the entire code (comment with ``` for short code, use Gist for long code)
  • The circuit diagram
  • Captured signals by an oscilloscope or a signal analyser (sigrok)

A question as a bug report is okay but we expect bug reporters to do their homework. The homework include:

For introductory C tutorials, see:

Submitting a fix

If you found a bug and a fix for it, please create a bug report before creating a Pull Request unless the bug is subtle, typos, or easy to reproduce and fix. Make sure to read Development Life Cycle as a Pull Request must meet the same standards documented in the section.

A GitHub Actions workflow, pr-labeler-action, is used to label PRs by branch name. Your fix branch should have prefixes defined in .github/pr-labeler.yml. Create a branch with one of the prefixes. If you are fixing a bug, your branch name should be bugfix-. The rest of branch name should be short, and descriptive. If the fix has related issues, the branch name should include them.

See also git branch name convention.

git checkout -b bugfix-issue-1

Change the branch name before creating a PR if your branch name does not follow the convention.

git branch --move bugfix-issue-1

Writing documentation

Even if you are not an author of the code in the repository, you can write documentation as a contribution.

Creating and maintaining FAQ entries is one of great examples. Have you encountered seemingly common issues while using a component? That might help others.

We encourage code authors to write documentation in the code so that the code and the documentation is always synced. However, sometimes they are not. Spotting such mistakes is always appreciated.

Not all contributors are native English speakers. If you are, please let us know ambiguity in the documentation, wrong usages of terms, and mistakes in English grammar. For this case, please create a Pull Request (creating an issue is optional).

Create a branch that documents features, or fixes existing documentations.

git checkout -b doc-foo

See also git branch name convention.

Suggesting enhancements

While we are not always able to write a driver for a chip, we still appreciate a request for new driver. It is more likely to happen when:

  • the chip is cool
  • the chip is easily available
  • the chip is affordable

Promoting the project

If you find the project useful, we are interested in what you did with esp-idf-lib, and how you did it.

  • Writing a blog post about your porject with esp-idf-lib
  • Mentioning the project in SNS

Writing code

If you can write a driver for new chip, that would be great. Please read Development Life Cycle.

Development Life Cycle

In this section, a typical development life cycle is explained.

Creating an Issue

If you are working on a new driver, or an existing one, please create an Issue, and assign the Issue to yourself.

esp-idf-lib aims at providing stable drivers for IC chips and general components. IC chips are stable, in that a chip is manufactured for a long time, retaining backward compatibilities. A driver for a chip usually requires minimal maintenance once the driver becomes stable. However, network protocols, graphics drivers, libraries for external services, are a moving-target. Standards will change, external services will evolve, user expectations will change, too. We think that such moving-targets should be maintained in a dedicated repository. Do you think your code is a moving target? It might be better to create a your own repository for the driver. If you are not sure, ask in the Issue.

Creating a feature branch in your fork and develop

Feature branch workflow is adopted in our development. Git Feature Branch Workflow by atlassian explains the workflow in details.

Fork the repository and clone it on your machine. See Fork a repo.

Create a feature branch in your fork from the master branch.

git checkout master

Check out the feature branch. The feature branch name should start with feat- or feature-.

git checkout -b feat-implement-foo

See also git branch name convention.

Write your code. Test the code in your physical test environment. Commit your changes and push them to your remote fork on GitHub.

components/example has an example component, and examples/example has an example application for the example component.

git add path/to/files
git commit -v
git push --set-upstream origin feat-implement-foo

See also Writing a commit message.

At this point, our CI workflows will run to test the changes. The test workflows include:

  • building the documentation,
  • building all examples for all supported targets with all supported esp-idf versions, and
  • linting code and documentation

You can see the test results in Actions page on your GitHub fork. To merge your changes to master branch, all the tests must pass.

Make sure you are working on the latest master of esp-idf-lib. To sync the master in your fork and the latest master of esp-idf-lib, run:

git checkout master
git fetch upstream
git reset --hard upstream/master

If your branch has many commits, consider git rebase to reduce the number of commits. This is especially useful when you are actively developing and the commit history has many trial-and-error commits.

git checkout feat-implement-foo
git rebase -i master
git push -f

Note that git rebase rewrites the commit history. You should avoid git rebase after you asked someone to review your code because the reviewer needs additional steps to ensure the review result is included.

C Code style

We use a style for source files based on LLVM Coding Standards except some cases, notably brace wrapping. Here is a brief list of the styles.

  • Use snake_case, not CamelCase
  • Use SNAKE_CASE in uppercase for macro name, e.g. MACRO_NAME.
  • Use spaces. The indent width is four
  • Use \n, or LF for line breaks
  • Use // for inline comments. Use /* */ for multi line comments after an empty line
  • Break before braces in most cases (functions, conditionals, control statements, etc)
  • Always check given arguments
  • Always check return code, return value, or errno
  • Return esp_err_t from functions where possible
  • Document public functions, data types, and macros in header files
  • Use suffix _t for typedef, e.g. foo_t
  • Use suffix _cb_t for function typedef, e.g.my_function_cb_t
  • Use suffix _s for struct, e.g. my_struct_s
  • Wrap numbers in macro definition with parenthesis, e.g. #define N_FOO (1)
  • Use #include <foo.h> for headers that are not part of the component, such as string.h, esp_log,h, and i2cdev.h. Use #include "foo.h" when the header is private, i.e. the header is the part of the component.

The style should be followed for all new code. In general, code can be considered "new code" when it makes up about 50% or more of the file(s) involved. This is enough to break precedents in the existing code and use the current style guidelines.

See an example source files under components/exmaple and, for complete rules, see .clang-format and the output of clang-format --dump-config.

New code will be tested in the CI, using clang-format (currently LLVM version 10).

To format your code without modifying the code, run:

clang-format10 components/example/example.c

To format your code in-place, run:

clang-format10 -i components/example/example.c

markdown Code style

We use the default markdownlint rules with some non-defaults. Our style can be found in .mdlstyle.rb.

Rule non-default options
MD003 - Header style use # for headers
MD007 - Unordered list indentation indent with 4 spaces
MD013 - Line length ignore line length in code blocks and tables
MD024 - Multiple headers with the same content allow_different_nesting is true
MD029 - Ordered list item prefix style is ordered, i.e. incremental numbers

In the CI, we use ruby version of markdownlint, or mdl gem, but markdownlint for node.js should also work.

To test markdown style of a file, you need:

  • ruby 2.6
  • bundler 2.x
bundle install
bundle exec mdl path/to/file

The output shows path to the file, line number, and the rule. An example output is shown below.

examples/led_strip_spi/README.md:30: MD040 Fenced code blocks should have a language specified

git branch name convention

We use the following convention for git branch name. Use one of branch name prefixes when creating a branch.

Branch name prefix Description
feat-, and feature- A feature branch that implements feature(s), or add enhancement(s) to existing code
fix-, and bugfix- A bug fix branch that fixes bug(s). The rest of the branch name should include issue number, such as fix-issue-1
ci- A branch that implements enhancement(s), or fixes issue(s) in CI
chore- A branch that does not affect code or its behavior, such as updating .gitignore
doc-, and documentation- Adding or updating documentation only, such as documenting undocumented features, or fixing existing documentation(s)

A GitHub Actions workflow automatically labels PRs depending on the branch name prefixes so that the PR is automatically included in release notes.

The rest of the branch name should be short, and descriptive. If your branch fixes, implements, or relates to, an Issue, include the Issue number. Say, if your branch fixes a bug reported Issue ${N}, the branch name should be fix-issue-${N} so that reviewer immediately understand there is a related Issue with your branch. Replace ${N} with the Issue number, such as fix-issue-123 when the Issue number is 123.

Typical issues you will face in developments

Your code assumes a single target, such as esp32. esp-idf-lib supports other targets, notably esp8266. Make sure the driver supports various other targets. If it cannot, such as the peripheral is not available on the target chip, your code should bailout during the build by using #error C preprocessor macro, and your driver must be excluded from the CI (TODO document how).

Your code assumes a single SDK. esp-idf-lib supports master and stable versions of esp-idf and ESP8266 RTOS SDK. Generally, the SDKs retain backward compatibilities, but sometimes not. Make sure to use if C preprocessor macro to support different versions. esp_idf_lib_helpers component can help you. ESP8266 RTOS SDK shares many functions and libraries, backported from esp-idf, but they are not identical. I2C drivers written with i2cdev should work fine on ESP32 and ESP8266, while SPI drivers need serious workarounds to support ESP8266. led_strip_spi attempted to support both, but you might want to write a different driver for each.

Your code assumes a single build method, such as idf.py. Although GNU make build method is considered as legacy, it is still a supported build method. The CI builds your code twice; with idf.py and with GNU make. Both must be successful. In ESP8266 RTOS SDK, idf.py is lagged behind from the one in esp-idf. For ESP8266 target, the CI builds examples with GNU make only.

Check return codes (most of functions in esp-idf), return values (e.g. malloc(3)), or errno (e.g. some standard C functions). Propagate the error by returning it from your function. An example:

#include <esp_err.h>
#include <esp_log.h>

esp_err_t do_something()
{
    esp_err_t err;

    err = foo();
    if (err != ESP_OK)
    {
        ESP_LOGE("bar", "foo(): %s", esp_err_to_name(err));
        goto fail;
    }
fail:
    return err;
}

Note that newer esp-idf supports useful macros for error handling, such as ESP_GOTO_ON_ERROR (see Error Handling), but older versions do not have them yet.

Check given arguments in functions, and return an appropriate error from one of predefined errors (see Error Codes Reference).

Writing a commit message

When you commit, prefix the first line of your commit message with foo:, where foo is the name of component you are working on. Sometimes, it is not possible because you are working on multiple components, i.e. fixing common bugs in multiple components. In such cases, use bugfix:. Other commonly used prefix words are:

  • feature: for features, or improvements, in multiple components
  • ci: for fixes or improvements in the CI process
  • doc: for fixes and improvements in the documentation

These prefix words are for conventional purposes. Use common sense and make the commit message clear so that others can understand what the change is.

The rest of the first line should start with a verb. Examples:

foo: fix typos

foo: resolve race condition in bar()

The first line should make sense when reading "When you merge this, it will $THE_FIRST_LINE".

The second line of the commit message must be an empty line.

In the rest of the commit message, write details of the change if necessary. Explain what it does and why. The lines in the commit message should be limited to 72 characters or less where possible.

Include a reference to an Issue when the commit fixes an Issue.

fixes #$ISSUE_NUMBER

When an Issue number or a Pull Request number is prefixed with certain keywords, the referenced Issue or Pull Request will be closed. See Linking a pull request to an issue using a keyword for the supported keywords.

Updating README.md

Each component has a .eil.yml file in its component directory. The file is a metadata file of the component. If you change the file, you need to update the README.md in the project root directory. The README.md is generated from the metadata and a template, README.md.erb. Generate README.md by:

bundle exec rake -C devtools readme > README.md

See also Metadata.md.

Creating a Pull Request

To test your code, you need to create a Pull Request. It is not practical to test code manually because you have to perform many tests. For instance, the number of tests is all targets (esp32, esp8266, esp32s2, etc) * build methods (make and idf.py) * supported esp-idf versions. Let the CI do it for you.

Before creating a Pull Request, make sure:

  1. You compiled the code and the build succeeded
  2. Functions, macros, data types are documented in the code
  3. An example application is provided under examples. In the directory, create a directory ${COMPONENT_NAME}/default. For instance, a component foo must have examples/foo/default. Create an example application in that directory.
  4. Update .github/labeler.yml. The component should have a label for it.
  5. You compiled the example code and the example application ran on a physical device as expected and documented
  6. All files are licensed under one of Acceptable Licenses by including the license at the top of file
  7. One of your commits in the feature branch, or the PR itself, mentions Issue number so that the Issue will be automatically closed when the PR is merged

When a PR is created, GitHub Actions workflows will:

  • label the PR with various labels, such as type of the PR (bug fix, or feature)
  • perform necessary tests depending on the changes (build the examples in a matrix if the source code is modified, build the documentation if files under docs are modified)

After the CI processes complete, you will see "All checks have passed" or some failures in the PR. To merge the PR, all checks must pass. Log is available from the link, Details, in the failed test.

If the PR does not pass the CI, update the branch with a fix. At this point, git rebase may be used. For instance, if a commit has a typo and one of the test fails because of syntax error, commit a fix of the syntax error and do git rebase to merge the fix into the original commit that has introduced the syntax error.

git add path/to/file
git commmit -v
git rebase -i master

-i, or --interactive, flag will launch a text editor where the history of the branch can be edited with commands. The buffer of the editor will look like:

pick f7f3f6d bugfix: fix issue foo
pick 310154e bugfix: fix a syntax error in f7f3f6d

The second commit, 310154e, should be part of the previous, f7f3f6d. To rewrite the commit history, replace pick with fixup.

pick f7f3f6d bugfix: fix issue foo
fixup 310154e bugfix: fix a syntax error in f7f3f6d

When you save and exit the editor, git rewrites the commit history as if 310154e was never committed. The commit 310154e is now part of f7f3f6d. If you don't save or modify the buffer, git will not rewrite the commit history.

git rebase can be used to tidy up the commits. To rebase or not to rebase depends on the nature of commits. A single commit per PR is preferred, but is not mandatory.

See also 7.6 Git Tools - Rewriting History.

When all the tests pass, ask the code owner to review the PR. The code owner can be found in .eli.yml file in the component directory. From this point, you should avoid to git rebase your feature branch. Otherwise, the reviewer would have to review the PR from scratch.

Developers who has write access to the repository will leave comments, ask rewrites, and merge the PR.

Licenses

We provide code that can be freely used, copied, modified, and distributed by anyone and for any purpose.

Acceptable licenses

We accept permissive licenses such as:

A list of licenses are available at SPDX License List.

Acceptable license for new code

New code is the one you own (you wrote it from scratch). The preferred license to be applied to new code is a simplified ISC License. The license must be included at the top in every files as long as practically possible. The following is a preferred wording of the license.

/*
 * SPDX-License-Identifier: ISC
 *
 * Copyright (c) YYYY YOUR NAME HERE <user@your.dom.ain>
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 */

Add SPDX-License-Identifier: $YOUR_LICENSE to your license header. $YOUR_LICENSE is a SPDX License Identifier.

Unacceptable licenses

We do NOT accept copyleft licenses such as:

  • GPL License
  • LGPL License
  • GNU Affero General Public License (AGPL)

We do NOT accept long licenses. A license is considered as long when it has more than four clauses.

We do NOT accept protective licenses that have additional restrictions, such as:

  • Apache license version 2 or later
  • various so-called Shareware or Freeware