Skip to content

Commit 446a313

Browse files
kattnikattni
authored
Merge pull request adafruit#600 from tannewt/clarify_property_comments
Clarify style of attribute comments in the Design Guide.
2 parents fac488f + 09bf06b commit 446a313

9 files changed

Lines changed: 177 additions & 97 deletions

File tree

‎docs/design_guide.rst‎

Lines changed: 137 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
Design Guide
22
============
33

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

813
Start libraries with the cookiecutter
914
-------------------------------------
1015

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

@@ -20,6 +25,10 @@ along with a setup.py, license, code of conduct and readme.
2025
2126
cookiecutter gh:adafruit/cookiecutter-adafruit-circuitpython
2227
28+
Cookiecutter will provide a series of prompts relating to the library and then create a new
29+
directory with all of the files. See `the CircuitPython cookiecutter README
30+
<https://github.com/adafruit/cookiecutter-adafruit-circuitpython#introduction>`_ for more details.
31+
2332
Module Naming
2433
-------------
2534

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

31-
Community created libraries should have the format ``CircuitPython_<name>`` and
40+
If the name would normally have a space, such as "Thermal Printer", use an underscore instead
41+
("Thermal_Printer"). This underscore will be used everywhere even when the separation between
42+
"adafruit" and "circuitpython" is done with a ``-``. Use the underscore in the cookiecutter prompts.
43+
44+
Community created libraries should have the repo format ``CircuitPython_<name>`` and
3245
not have the ``adafruit_`` module or package prefix.
3346

3447
Both should have the CircuitPython repository topic on GitHub.
@@ -90,7 +103,7 @@ Verify your device
90103
--------------------------------------------------------------------------------
91104

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

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

141+
When adding a new module for additional functionality related to a CPython
142+
module do NOT simply prefix it with u. This is not a large enough differentiation
143+
from CPython. This is the MicroPython convention and they use u* modules
144+
interchangeably with the CPython name. This is confusing. Instead, think up a
145+
new name that is related to the extra functionality you are adding.
146+
147+
For example, storage mounting and unmounting related functions were moved from
148+
``uos`` into a new `storage` module. Terminal related functions were moved into
149+
`multiterminal`. These names better match their functionality and do not
150+
conflict with CPython names. Make sure to check that you don't conflict with
151+
CPython libraries too. That way we can port the API to CPython in the future.
152+
128153
Example
129154
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
130155

@@ -164,61 +189,74 @@ After the license comment::
164189
Class description
165190
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
166191

167-
Documenting what the object does::
192+
At the class level document what class does and how to initialize it::
168193

169194
class DS3231:
170-
"""Interface to the DS3231 RTC."""
195+
"""DS3231 real-time clock.
171196

172-
Renders as:
197+
:param ~busio.I2C i2c_bus: The I2C bus the DS3231 is connected to.
198+
:param int address: The I2C address of the device.
199+
"""
173200

174-
.. py:class:: DS3231
175-
:noindex:
201+
def __init__(self, i2c_bus, address=0x40):
202+
self._i2c = i2c_bus
176203

177-
Interface to the DS3231 RTC.
178204

179-
Data descriptor description
180-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
205+
Renders as:
181206

182-
Comment is after even though its weird::
207+
.. py:class:: DS3231(i2c_bus, address=64)
208+
:noindex:
183209

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

187-
Renders as:
212+
:param ~busio.I2C i2c_bus: The I2C bus the DS3231 is connected to.
213+
:param int address: The I2C address of the device.
188214

189-
.. py:attribute:: lost_power
190-
:noindex:
215+
Attributes
216+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
191217

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

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

197-
First line after the method definition::
228+
Instance attributes
229+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
198230

199-
def turn_right(self, degrees):
200-
"""Turns the bot ``degrees`` right.
231+
Comment comes from after the assignment::
201232

202-
:param float degrees: Degrees to turn right
233+
def __init__(self, drive_mode):
234+
self.drive_mode = drive_mode
235+
"""
236+
The pin drive mode. One of:
237+
238+
- `digitalio.DriveMode.PUSH_PULL`
239+
- `digitalio.DriveMode.OPEN_DRAIN`
203240
"""
204241

205242
Renders as:
206243

207-
.. py:method:: turn_right(degrees)
208-
:noindex:
244+
.. py:attribute:: drive_mode
245+
:noindex:
209246

210-
Turns the bot ``degrees`` right.
247+
The pin drive mode. One of:
211248

212-
:param float degrees: Degrees to turn right
249+
- `digitalio.DriveMode.PUSH_PULL`
250+
- `digitalio.DriveMode.OPEN_DRAIN`
213251

214252
Property description
215-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
253+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
216254

217255
Comment comes from the getter::
218256

219257
@property
220258
def datetime(self):
221-
"""The current date and time"""
259+
"""The current date and time as a `time.struct_time`."""
222260
return self.datetime_register
223261

224262
@datetime.setter
@@ -228,9 +266,65 @@ Comment comes from the getter::
228266
Renders as:
229267

230268
.. py:attribute:: datetime
269+
:noindex:
270+
271+
The current date and time as a `time.struct_time`.
272+
273+
Read-only example::
274+
275+
@property
276+
def temperature(self):
277+
"""
278+
The current temperature in degrees Celsius. (read-only)
279+
280+
The device may require calibration to get accurate readings.
281+
"""
282+
return self._read(TEMPERATURE)
283+
284+
285+
Renders as:
286+
287+
.. py:attribute:: temperature
288+
:noindex:
289+
290+
The current temperature in degrees Celsius. (read-only)
291+
292+
The device may require calibration to get accurate readings.
293+
294+
Data descriptor description
295+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
296+
297+
Comment is after the definition::
298+
299+
lost_power = i2c_bit.RWBit(0x0f, 7)
300+
"""True if the device has lost power since the time was set."""
301+
302+
Renders as:
303+
304+
.. py:attribute:: lost_power
305+
:noindex:
306+
307+
True if the device has lost power since the time was set.
308+
309+
Method description
310+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
311+
312+
First line after the method definition::
313+
314+
def turn_right(self, degrees):
315+
"""Turns the bot ``degrees`` right.
316+
317+
:param float degrees: Degrees to turn right
318+
"""
319+
320+
Renders as:
321+
322+
.. py:method:: turn_right(degrees)
231323
:noindex:
232324

233-
The current date and time
325+
Turns the bot ``degrees`` right.
326+
327+
:param float degrees: Degrees to turn right
234328

235329
Use BusDevice
236330
--------------------------------------------------------------------------------
@@ -397,14 +491,14 @@ properties.
397491
+-----------------------+-----------------------+-------------------------------------------------------------------------+
398492
| ``datetime`` | time.struct | date and time |
399493
+-----------------------+-----------------------+-------------------------------------------------------------------------+
400-
401-
Common APIs
402-
--------------------------------------------------------------------------------
403-
404-
Outside of sensors, having common methods amongst drivers for similar devices
405-
such as devices can be really useful. Its early days however. For now, try to
406-
adhere to guidelines in this document. Once a design is settled on, add it as a
407-
subsection to this one.
494+
| ``duty_cycle`` | int | 16-bit PWM duty cycle (regardless of output resolution) |
495+
+-----------------------+-----------------------+-------------------------------------------------------------------------+
496+
| ``frequency`` | int | Hertz |
497+
+-----------------------+-----------------------+-------------------------------------------------------------------------+
498+
| ``value`` | bool | Digital logic |
499+
+-----------------------+-----------------------+-------------------------------------------------------------------------+
500+
| ``value`` | int | 16-bit Analog value, unit-less |
501+
+-----------------------+-----------------------+-------------------------------------------------------------------------+
408502

409503
Adding native modules
410504
--------------------------------------------------------------------------------

‎shared-bindings/analogio/AnalogIn.c‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(analogio_analogin___exit___obj, 4, 4,
106106

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

131128
//| .. attribute:: reference_voltage
132129
//|
133-
//| The maximum voltage measurable. Also known as the reference voltage.
134-
//|
135-
//| :return: the reference voltage
136-
//| :rtype: float
130+
//| The maximum voltage measurable (also known as the reference voltage) as a
131+
//| `float` in Volts.
137132
//|
138133
STATIC mp_obj_t analogio_analogin_obj_get_reference_voltage(mp_obj_t self_in) {
139134
analogio_analogin_obj_t *self = MP_OBJ_TO_PTR(self_in);

‎shared-bindings/analogio/AnalogOut.c‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,10 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(analogio_analogout___exit___obj, 4, 4
105105

106106
//| .. attribute:: value
107107
//|
108-
//| The value on the analog pin. The value must be between 0 and 65535
109-
//| inclusive (16-bit). Even if the underlying digital to analog converter
110-
//| is lower resolution, the input must be scaled to be 16-bit.
111-
//|
112-
//| :return: the last value written
113-
//| :rtype: int
108+
//| The value on the analog pin between 0 and 65535 inclusive (16-bit). (write-only)
114109
//|
110+
//| Even if the underlying digital to analog converter (DAC) is lower
111+
//| resolution, the value is 16-bit.
115112
STATIC mp_obj_t analogio_analogout_obj_set_value(mp_obj_t self_in, mp_obj_t value) {
116113
analogio_analogout_obj_t *self = MP_OBJ_TO_PTR(self_in);
117114
raise_error_if_deinited(common_hal_analogio_analogout_deinited(self));

‎shared-bindings/audioio/AudioOut.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(audioio_audioout_stop_obj, audioio_audioout_obj_stop);
187187

188188
//| .. attribute:: playing
189189
//|
190-
//| True when the audio sample is being output.
190+
//| True when the audio sample is being output. (read-only)
191191
//|
192192
STATIC mp_obj_t audioio_audioout_obj_get_playing(mp_obj_t self_in) {
193193
audioio_audioout_obj_t *self = MP_OBJ_TO_PTR(self_in);

‎shared-bindings/digitalio/DigitalInOut.c‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,10 @@ const mp_obj_property_t digitalio_digitalinout_value_obj = {
253253

254254
//| .. attribute:: drive_mode
255255
//|
256-
//| Get or set the pin drive mode.
256+
//| The pin drive mode. One of:
257+
//|
258+
//| - `digitalio.DriveMode.PUSH_PULL`
259+
//| - `digitalio.DriveMode.OPEN_DRAIN`
257260
//|
258261
STATIC mp_obj_t digitalio_digitalinout_obj_get_drive_mode(mp_obj_t self_in) {
259262
digitalio_digitalinout_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -295,10 +298,13 @@ const mp_obj_property_t digitalio_digitalio_drive_mode_obj = {
295298

296299
//| .. attribute:: pull
297300
//|
298-
//| Get or set the pin pull. Values may be `digitalio.Pull.UP`,
299-
//| `digitalio.Pull.DOWN` or ``None``.
301+
//| The pin pull direction. One of:
302+
//|
303+
//| - `digitalio.Pull.UP`
304+
//| - `digitalio.Pull.DOWN`
305+
//| - `None`
300306
//|
301-
//| :raises AttributeError: if the direction is ~`digitalio.Direction.OUTPUT`.
307+
//| :raises AttributeError: if `direction` is :py:data:`~digitalio.Direction.OUTPUT`.
302308
//|
303309
STATIC mp_obj_t digitalio_digitalinout_obj_get_pull(mp_obj_t self_in) {
304310
digitalio_digitalinout_obj_t *self = MP_OBJ_TO_PTR(self_in);

0 commit comments

Comments
 (0)