Skip to content

doc and cleanup pass #32

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

Merged
merged 4 commits into from
Nov 15, 2019
Merged

doc and cleanup pass #32

merged 4 commits into from
Nov 15, 2019

Conversation

dhalbert
Copy link
Collaborator

I made a pass over all the files to do a more thorough code review, made some changes, and updated some doc. In general:

-- Use of kwargs was mostly dropped in favor of explicit arg names.
-- In a few places, the fact that an object was from _bleio was made more explicit.
-- Some pylint overrides were removed by renaming variables, etc.
-- A couple of pylint warnings were dealt with.
-- _bleio.Attribute and _bleio.Characteristic CONSTANT values were duplicated in the library, so that user code and library code does not need to refer to these directly.
-- 8, 16, and 32 bit signed and unsigned characteristic classes were created.

  • Fixed an apparent bug in time_struct stuff, caught by @jerryneedell.

Tested ble_uart_echo_test.py andble_detailed_scan.py.
Tested ble_demo_central.py and ble_demo_periph.py working together on a pair of CPB's.

There is one minor corresponding fix to circuitpython which I'll submit a PR for shortly.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

A few comments from the first couple files. Will finish later.

@jerryneedell
Copy link
Contributor

Don’t these references to “core” have to be fixed?

@dhalbert
Copy link
Collaborator Author

@jerryneedell Yes, thanks, that code wasn't used, so it didn't get vetted.

@jerryneedell
Copy link
Contributor

jerryneedell commented Nov 14, 2019

hmm -- tried simpletest

Press any key to enter the REPL. Use CTRL-D to reload.
Adafruit CircuitPython 5.0.0-alpha.5-80-g166518fc9 on 2019-11-13; Adafruit Feather nRF52840 Express with nRF52840
>>> import ble_simpletest
scanning
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "ble_simpletest.py", line 10, in <module>
  File "adafruit_ble/__init__.py", line 198, in start_scan
TypeError: can't convert NoneType to float
>>> 

@dhalbert
Copy link
Collaborator Author

@jerryneedell This is fixed by adafruit/circuitpython#2287, so you need a newer CPy.

@jerryneedell
Copy link
Contributor

Ah -- so it is!! Sorry - I missed that commit to CP. works fine

from ..characteristics.string import StringCharacteristic
from ..characteristics.core import StructCharacteristic
from ..characteristics import StructCharacteristic
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI -- each of these needs one more "." ...characteristics

Copy link
Contributor

Choose a reason for hiding this comment

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

Still returns all 0's ,but with that change, it does not throw any errors ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, I will push another change, but what returns all 0's?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been running this -- modified from your old version

"""CTS demo
"""

import board
import displayio
import terminalio
import time

import adafruit_displayio_ssd1306
from adafruit_display_text import label
from adafruit_ble import BLERadio
from adafruit_ble.advertising.standard import ProvideServicesAdvertisement
from adafruit_ble.services.standard.standard import CurrentTimeService

ble = BLERadio()
cts = CurrentTimeService()
advertisement = ProvideServicesAdvertisement(cts)



displayio.release_displays()

i2c = board.I2C()
display_bus = displayio.I2CDisplay(i2c, device_address=0x3c)
display = adafruit_displayio_ssd1306.SSD1306(display_bus, width=128, height=32)

FONT = terminalio.FONT
WHITE = 0xffffff
CONNECTING = "Connecting..."
text = CONNECTING

text_area = label.Label(FONT, max_glyphs=50, text=text, color=WHITE)

text_area.x = 0
text_area.y = 10


display.show(text_area)

while True:
    text_area.text = CONNECTING
    ble.start_advertising(advertisement)
    while not ble.connected:
        pass

    while ble.connected:
        print(cts.struct_time)
        year, month, day, hour, minute, second, _, _, _ = cts.current_time
        text_area.text = '{}-{}-{} {:02}:{:02}:{:02}'.format(year, month, day, hour, minute, second)
        time.sleep(5)

I don't get the pairing option on Settings on my iPhone, but if I connect via the BLE Conect App it connects and but the time struct is all 0's

Press any key to enter the REPL. Use CTRL-D to reload.
Adafruit CircuitPython 5.0.0-alpha.5-84-g4c55431bb on 2019-11-14; Adafruit Feather nRF52840 Express with nRF52840
>>> import cts
struct_time(tm_year=0, tm_mon=0, tm_mday=0, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=-1, tm_yday=-1, tm_isdst=-1)

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not have to be resolved for this PR -- no need to hold things up.

@jerryneedell
Copy link
Contributor

Tested central/periph demo on CPBs and uart_echo_test,ble_simpletest and ble_detailed_scan on feather_nrf52840. Latest fixes to standard.py fix the errors I was seeing - still experimenting with CTS but not working yet.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for the polish.

@dhalbert dhalbert merged commit e6679ab into adafruit:master Nov 15, 2019
@dhalbert dhalbert deleted the doc-and-cleanup branch November 15, 2019 01:05
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Nov 20, 2019
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1305 to 1.0.3 from 1.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1305#5 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_RGB_Display to 3.6.0 from 3.5.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#51 from makermelissa/master
  > Merge pull request adafruit/Adafruit_CircuitPython_RGB_Display#47 from Johennes/feature/numpy

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE to 3.0.0 from 1.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#34 from dhalbert/check-cpy-version
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#32 from dhalbert/doc-and-cleanup
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#31 from dhalbert/char-fixes-and-float
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#30 from tannewt/api_rework
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#29 from adafruit/dherrada-patch-1
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#25 from kattni/plotter-code
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#23 from kattni/update-color-picker
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#22 from kattni/update-color-picker
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#21 from kattni/cpb-color-picker
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#18 from dhalbert/bleio-api-revamp
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#17 from dhalbert/demo-central
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#16 from dhalbert/pairing
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE#15 from dhalbert/python-advertisement-data
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.

3 participants