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

Clarify style of attribute comments in the Design Guide. #600

Merged
merged 3 commits into from
Feb 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 137 additions & 43 deletions docs/design_guide.rst
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
Design Guide
============

MicroPython has created a great foundation to build upon and to make it even
better for beginners we've created CircuitPython. This guide covers a number of
ways the core and libraries are geared towards beginners.
This guide covers a variety of development practices for CircuitPython core and library APIs. These
APIs are both `built-into CircuitPython
<https://github.com/adafruit/circuitpython/tree/master/shared-bindings>`_ and those that are
`distributed on GitHub <https://github.com/search?utf8=%E2%9C%93&q=topic%3Acircuitpython&type=>`_
and in the `Adafruit <https://github.com/adafruit/Adafruit_CircuitPython_Bundle>`_ and `Community
<https://github.com/adafruit/CircuitPython_Community_Bundle/>`_ bundles. Consistency with these
practices ensures that beginners can learn a pattern once and apply it throughout the CircuitPython
ecosystem.

Start libraries with the cookiecutter
-------------------------------------

Cookiecutter is a cool tool that lets you bootstrap a new repo based on another
repo. We've made one `here <https://github.com/adafruit/cookiecutter-adafruit-circuitpython>`_
Cookiecutter is a tool that lets you bootstrap a new repo based on another repo.
We've made one `here <https://github.com/adafruit/cookiecutter-adafruit-circuitpython>`_
for CircuitPython libraries that include configs for Travis CI and ReadTheDocs
along with a setup.py, license, code of conduct and readme.

Expand All @@ -20,6 +25,10 @@ along with a setup.py, license, code of conduct and readme.

cookiecutter gh:adafruit/cookiecutter-adafruit-circuitpython

Cookiecutter will provide a series of prompts relating to the library and then create a new
directory with all of the files. See `the CircuitPython cookiecutter README
<https://github.com/adafruit/cookiecutter-adafruit-circuitpython#introduction>`_ for more details.

Module Naming
-------------

Expand All @@ -28,7 +37,11 @@ Adafruit funded libraries should be under the
``Adafruit_CircuitPython_<name>`` and have a corresponding ``adafruit_<name>``
directory (aka package) or ``adafruit_<name>.py`` file (aka module).

Community created libraries should have the format ``CircuitPython_<name>`` and
If the name would normally have a space, such as "Thermal Printer", use an underscore instead
("Thermal_Printer"). This underscore will be used everywhere even when the separation between
"adafruit" and "circuitpython" is done with a ``-``. Use the underscore in the cookiecutter prompts.

Community created libraries should have the repo format ``CircuitPython_<name>`` and
not have the ``adafruit_`` module or package prefix.

Both should have the CircuitPython repository topic on GitHub.
Expand Down Expand Up @@ -90,7 +103,7 @@ Verify your device
--------------------------------------------------------------------------------

Whenever possible, make sure device you are talking to is the device you expect.
If not, raise a ValueError. Beware that I2C addresses can be identical on
If not, raise a RuntimeError. Beware that I2C addresses can be identical on
different devices so read registers you know to make sure they match your
expectation. Validating this upfront will help catch mistakes.

Expand Down Expand Up @@ -125,6 +138,18 @@ modules to add extra functionality. By distinguishing API boundaries at modules
you increase the likelihood that incorrect expectations are found on import and
not randomly during runtime.

When adding a new module for additional functionality related to a CPython
module do NOT simply prefix it with u. This is not a large enough differentiation
from CPython. This is the MicroPython convention and they use u* modules
interchangeably with the CPython name. This is confusing. Instead, think up a
new name that is related to the extra functionality you are adding.

For example, storage mounting and unmounting related functions were moved from
``uos`` into a new `storage` module. Terminal related functions were moved into
`multiterminal`. These names better match their functionality and do not
conflict with CPython names. Make sure to check that you don't conflict with
CPython libraries too. That way we can port the API to CPython in the future.

Example
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down Expand Up @@ -164,61 +189,74 @@ After the license comment::
Class description
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Documenting what the object does::
At the class level document what class does and how to initialize it::

class DS3231:
"""Interface to the DS3231 RTC."""
"""DS3231 real-time clock.

Renders as:
:param ~busio.I2C i2c_bus: The I2C bus the DS3231 is connected to.
:param int address: The I2C address of the device.
"""

.. py:class:: DS3231
:noindex:
def __init__(self, i2c_bus, address=0x40):
self._i2c = i2c_bus

Interface to the DS3231 RTC.

Data descriptor description
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Renders as:

Comment is after even though its weird::
.. py:class:: DS3231(i2c_bus, address=64)
:noindex:

lost_power = i2c_bit.RWBit(0x0f, 7)
"""True if the device has lost power since the time was set."""
DS3231 real-time clock.

Renders as:
:param ~busio.I2C i2c_bus: The I2C bus the DS3231 is connected to.
:param int address: The I2C address of the device.

.. py:attribute:: lost_power
:noindex:
Attributes
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

True if the device has lost power since the time was set.
Attributes are state on objects. (See `Getters/Setters` above for more discussion
about when to use them.) They can be defined internally in a number of different
ways. Each approach is enumerated below with an explanation of where the comment
goes.

Method description
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Regardless of how the attribute is implemented, it should have a short
description of what state it represents including the type, possible values and/or
units. It should be marked as ``(read-only)`` or ``(write-only)`` at the end of
the first line for attributes that are not both readable and writable.

First line after the method definition::
Instance attributes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

def turn_right(self, degrees):
"""Turns the bot ``degrees`` right.
Comment comes from after the assignment::

:param float degrees: Degrees to turn right
def __init__(self, drive_mode):
self.drive_mode = drive_mode
"""
The pin drive mode. One of:

- `digitalio.DriveMode.PUSH_PULL`
- `digitalio.DriveMode.OPEN_DRAIN`
"""

Renders as:

.. py:method:: turn_right(degrees)
:noindex:
.. py:attribute:: drive_mode
:noindex:

Turns the bot ``degrees`` right.
The pin drive mode. One of:

:param float degrees: Degrees to turn right
- `digitalio.DriveMode.PUSH_PULL`
- `digitalio.DriveMode.OPEN_DRAIN`

Property description
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment comes from the getter::

@property
def datetime(self):
"""The current date and time"""
"""The current date and time as a `time.struct_time`."""
return self.datetime_register

@datetime.setter
Expand All @@ -228,9 +266,65 @@ Comment comes from the getter::
Renders as:

.. py:attribute:: datetime
:noindex:

The current date and time as a `time.struct_time`.

Read-only example::

@property
def temperature(self):
"""
The current temperature in degrees Celsius. (read-only)

The device may require calibration to get accurate readings.
"""
return self._read(TEMPERATURE)


Renders as:

.. py:attribute:: temperature
:noindex:

The current temperature in degrees Celsius. (read-only)

The device may require calibration to get accurate readings.

Data descriptor description
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment is after the definition::

lost_power = i2c_bit.RWBit(0x0f, 7)
"""True if the device has lost power since the time was set."""

Renders as:

.. py:attribute:: lost_power
:noindex:

True if the device has lost power since the time was set.

Method description
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

First line after the method definition::

def turn_right(self, degrees):
"""Turns the bot ``degrees`` right.

:param float degrees: Degrees to turn right
"""

Renders as:

.. py:method:: turn_right(degrees)
:noindex:

The current date and time
Turns the bot ``degrees`` right.

:param float degrees: Degrees to turn right

Use BusDevice
--------------------------------------------------------------------------------
Expand Down Expand Up @@ -397,14 +491,14 @@ properties.
+-----------------------+-----------------------+-------------------------------------------------------------------------+
| ``datetime`` | time.struct | date and time |
+-----------------------+-----------------------+-------------------------------------------------------------------------+

Common APIs
--------------------------------------------------------------------------------

Outside of sensors, having common methods amongst drivers for similar devices
such as devices can be really useful. Its early days however. For now, try to
adhere to guidelines in this document. Once a design is settled on, add it as a
subsection to this one.
| ``duty_cycle`` | int | 16-bit PWM duty cycle (regardless of output resolution) |
+-----------------------+-----------------------+-------------------------------------------------------------------------+
| ``frequency`` | int | Hertz |
+-----------------------+-----------------------+-------------------------------------------------------------------------+
| ``value`` | bool | Digital logic |
+-----------------------+-----------------------+-------------------------------------------------------------------------+
| ``value`` | int | 16-bit Analog value, unit-less |
+-----------------------+-----------------------+-------------------------------------------------------------------------+

Adding native modules
--------------------------------------------------------------------------------
Expand Down
15 changes: 5 additions & 10 deletions shared-bindings/analogio/AnalogIn.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(analogio_analogin___exit___obj, 4, 4,

//| .. attribute:: value
//|
//| Read the value on the analog pin and return it. The returned value
//| will be between 0 and 65535 inclusive (16-bit). Even if the underlying
//| analog to digital converter (ADC) is lower resolution, the result will
//| be scaled to be 16-bit.
//| The value on the analog pin between 0 and 65535 inclusive (16-bit). (read-only)
//|
//| :return: the data read
//| :rtype: int
//| Even if the underlying analog to digital converter (ADC) is lower
//| resolution, the value is 16-bit.
//|
STATIC mp_obj_t analogio_analogin_obj_get_value(mp_obj_t self_in) {
analogio_analogin_obj_t *self = MP_OBJ_TO_PTR(self_in);
Expand All @@ -130,10 +127,8 @@ const mp_obj_property_t analogio_analogin_value_obj = {

//| .. attribute:: reference_voltage
//|
//| The maximum voltage measurable. Also known as the reference voltage.
//|
//| :return: the reference voltage
//| :rtype: float
//| The maximum voltage measurable (also known as the reference voltage) as a
//| `float` in Volts.
//|
STATIC mp_obj_t analogio_analogin_obj_get_reference_voltage(mp_obj_t self_in) {
analogio_analogin_obj_t *self = MP_OBJ_TO_PTR(self_in);
Expand Down
9 changes: 3 additions & 6 deletions shared-bindings/analogio/AnalogOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(analogio_analogout___exit___obj, 4, 4

//| .. attribute:: value
//|
//| The value on the analog pin. The value must be between 0 and 65535
//| inclusive (16-bit). Even if the underlying digital to analog converter
//| is lower resolution, the input must be scaled to be 16-bit.
//|
//| :return: the last value written
//| :rtype: int
//| The value on the analog pin between 0 and 65535 inclusive (16-bit). (write-only)
//|
//| Even if the underlying digital to analog converter (DAC) is lower
//| resolution, the value is 16-bit.
STATIC mp_obj_t analogio_analogout_obj_set_value(mp_obj_t self_in, mp_obj_t value) {
analogio_analogout_obj_t *self = MP_OBJ_TO_PTR(self_in);
raise_error_if_deinited(common_hal_analogio_analogout_deinited(self));
Expand Down
2 changes: 1 addition & 1 deletion shared-bindings/audioio/AudioOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(audioio_audioout_stop_obj, audioio_audioout_obj_stop);

//| .. attribute:: playing
//|
//| True when the audio sample is being output.
//| True when the audio sample is being output. (read-only)
//|
STATIC mp_obj_t audioio_audioout_obj_get_playing(mp_obj_t self_in) {
audioio_audioout_obj_t *self = MP_OBJ_TO_PTR(self_in);
Expand Down
14 changes: 10 additions & 4 deletions shared-bindings/digitalio/DigitalInOut.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,10 @@ const mp_obj_property_t digitalio_digitalinout_value_obj = {

//| .. attribute:: drive_mode
//|
//| Get or set the pin drive mode.
//| The pin drive mode. One of:
//|
//| - `digitalio.DriveMode.PUSH_PULL`
//| - `digitalio.DriveMode.OPEN_DRAIN`
//|
STATIC mp_obj_t digitalio_digitalinout_obj_get_drive_mode(mp_obj_t self_in) {
digitalio_digitalinout_obj_t *self = MP_OBJ_TO_PTR(self_in);
Expand Down Expand Up @@ -295,10 +298,13 @@ const mp_obj_property_t digitalio_digitalio_drive_mode_obj = {

//| .. attribute:: pull
//|
//| Get or set the pin pull. Values may be `digitalio.Pull.UP`,
//| `digitalio.Pull.DOWN` or ``None``.
//| The pin pull direction. One of:
//|
//| - `digitalio.Pull.UP`
//| - `digitalio.Pull.DOWN`
//| - `None`
//|
//| :raises AttributeError: if the direction is ~`digitalio.Direction.OUTPUT`.
//| :raises AttributeError: if `direction` is :py:data:`~digitalio.Direction.OUTPUT`.
//|
STATIC mp_obj_t digitalio_digitalinout_obj_get_pull(mp_obj_t self_in) {
digitalio_digitalinout_obj_t *self = MP_OBJ_TO_PTR(self_in);
Expand Down
Loading