-
Notifications
You must be signed in to change notification settings - Fork 57
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
Conversation
There was a problem hiding this 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.
Don’t these references to “core” have to be fixed?
|
@jerryneedell Yes, thanks, that code wasn't used, so it didn't get vetted. |
hmm -- tried simpletest
|
@jerryneedell This is fixed by adafruit/circuitpython#2287, so you need a newer CPy. |
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 ;-)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
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. |
There was a problem hiding this 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.
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
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.
time_struct
stuff, caught by @jerryneedell.Tested
ble_uart_echo_test.py
andble_detailed_scan.py
.Tested
ble_demo_central.py
andble_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.