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

Bug in isCrownstoneInSetupMode? #4

Open
mrquincle opened this issue Feb 2, 2021 · 14 comments
Open

Bug in isCrownstoneInSetupMode? #4

mrquincle opened this issue Feb 2, 2021 · 14 comments

Comments

@mrquincle
Copy link

The function isCrownstoneInSetupMode doesn't seem to work.

@vliedel
Copy link
Contributor

vliedel commented Feb 15, 2021

So i've been testing a bit, and have some questions:

  • What lib scan backend do you use (bluepy or Aio)? I only tested with the default, Bluepy.
  • What parameters did you use for the call?
  • What bluetooth device did you use?

@mrquincle
Copy link
Author

mrquincle commented Feb 15, 2021

This is all information:

#!/usr/bin/env python3

import time
import argparse
import json

from crownstone_ble import CrownstoneBle, ScanBackends

parser = argparse.ArgumentParser(description='Search for any Crownstone and print their information')
parser.add_argument('--hciIndex', dest='hciIndex', metavar='I', type=int, nargs='?', default=0,
        help='The hci-index of the BLE chip')
parser.add_argument('keyFile',
        help='The json file with key information, expected values: admin, member, guest, basic,' +
        'serviceDataKey, localizationKey, meshApplicationKey, and meshNetworkKey')
parser.add_argument('macAddress', type=str,
        help='The bluetooth MAC address of the Crownstone to connect to.')

args = parser.parse_args()

print("===========================================\n\ncsutil\n\n===========================================")

# Initialize the Bluetooth Core.
core = CrownstoneBle(hciIndex=args.hciIndex)
core.loadSettingsFromFile(args.keyFile)

address = args.macAddress

def setup_test():
    print("Is Crownstone in setup mode at address:", address)

    # Does not seem to work properly
    if core.isCrownstoneInSetupMode(address):
        print("Crownstone is in setup mode")
    else:
        print("Crownstone is not in setup mode")

    print("Sleep to be easy on the lib")
    time.sleep(10)

setup_test()

# clean up all pending processes
print("Core shutdown")
core.shutDown()

@vliedel
Copy link
Contributor

vliedel commented Feb 15, 2021

That's a lot of code for me to search through. My conclusions:

  • You seem to use the default backend.
  • You don't call isCrownstoneInSetupMode at all.
  • I can't see your hardware setup from code.

@mrquincle
Copy link
Author

  • Yes, I use everything default.
  • I have commented out isCrownstoneInSetupMode because it fails.
  • I'm using the nRF52832 dev kit, PCA10040.

@mrquincle
Copy link
Author

Just to make sure that the bug report is complete:

  • The code reports the Crownstone to be not in setup mode, while the Crownstone app shows that it is in setup mode.
  • The code above uses a json file with keys that can be used to actually control the Crownstone. I'm not sure why I need keys to see if a Crownstone is in setup mode, but if I don't call loadSettingsFromFile, I get the following error:
Traceback (most recent call last):
  File "./is_in_setup.py", line 40, in <module>
    setup_test()
  File "./is_in_setup.py", line 32, in setup_test
    if core.isCrownstoneInSetupMode(address):
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_ble/core/CrownstoneBle.py", line 112, in isCrownstoneInSetupMode
    self.ble.startScanning(scanDuration=scanDuration)
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_ble/core/ble_modules/BleHandler.py", line 114, in startScanning
    self.scanner.process(processInterval)
  File "/home/anne/.local/lib/python3.8/site-packages/bluepy/btle.py", line 842, in process
    self.delegate.handleDiscovery(dev, (dev.updateCount <= 1), isNewData)
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_ble/core/bluetooth_delegates/ScanDelegate.py", line 21, in handleDiscovery
    self.parsePayload(dev.addr, dev.rssi, nameText, valueText)
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_ble/core/bluetooth_delegates/ScanDelegate.py", line 29, in parsePayload
    advertisement.decrypt(self.settings.serviceDataKey)
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_core/packets/Advertisement.py", line 55, in decrypt
    self.serviceData.decrypt(key)
  File "/home/anne/.local/lib/python3.8/site-packages/crownstone_core/packets/ServiceData.py", line 167, in decrypt
    if self.validData and len(self.encryptedData) == 16 and len(keyHexString) >= 16:
TypeError: object of type 'NoneType' has no len()

The given code I've called is_in_setup.py and is called with:

./is_in_setup.py keys.json C2:81:75:46:D8:F1

@vliedel
Copy link
Contributor

vliedel commented Feb 15, 2021

I tried your script, and it works fine here. Your bluetooth hardware is probably not scanning well or something, you can try with a longer scan duration.
Not being able to tell whether there was a scan at all, is the real issue here, I think.

Regarding the lib not working without keys, you can make a new issue for that. I think it's silly for the lib to crash when keys are not set.

@mrquincle
Copy link
Author

Hi, may I ask for an option to do it with or without scanning?

@mrquincle
Copy link
Author

Suggestion:

isCrownstoneInSetupMode(..., use_scanning = true)

However, even better:

connect(...)
# if connected
isInSetupMode()
disconnect

Then I can handle failures in connection from knowledge about setup mode. Would be awesome!

@vliedel
Copy link
Contributor

vliedel commented Feb 15, 2021

Yes, fully agree that would be a nice function to have. It would probably be used internally as well, and give you some NOT_IN_SETUP_MODE error or so for the setup() command.

@mrquincle
Copy link
Author

Argh!!!!

 ./is_in_setup.py ~/.crownstone/keys.json C2:81:75:46:D8:F1

Search Results: {'name': 'crwn', 'address': 'c2:81:75:46:d8:f1', 'rssi': -51, 'setupMode': True, 'id': 0}
Starting setup on  c2:81:75:46:d8:f1
Is Crownstone in setup mode at address: C2:81:75:46:D8:F1
Crownstone is not in setup mode
Sleep to be easy on the lib
Core shutdown

Lower case mac address:

./is_in_setup.py ~/.crownstone/keys.json c2:81:75:46:d8:f1
Search Results: {'name': 'crwn', 'address': 'c2:81:75:46:d8:f1', 'rssi': -53, 'setupMode': True, 'id': 0}
Starting setup on  c2:81:75:46:d8:f1
Is Crownstone in setup mode at address: c2:81:75:46:d8:f1
Crownstone is in setup mode
Sleep to be easy on the lib
Core shutdown

@mrquincle
Copy link
Author

Fix in this script is of course:

address = args.macAddress.lower()

However, it would be great to have the lib case-insensitive.

@vliedel
Copy link
Contributor

vliedel commented Feb 15, 2021

Right, so the function works :)

I wrote down some improvements for the library, which will be done later.

@vliedel vliedel closed this as completed Feb 15, 2021
@mrquincle mrquincle reopened this Feb 16, 2021
@mrquincle
Copy link
Author

The case sensitivity is a bug, I'm afraid.

@AlexDM0
Copy link
Contributor

AlexDM0 commented Feb 16, 2021

Let's split a few things up.
1 - the initial issue was that the isCrownstoneInSetupMode would not differentiate between "its not" and "I dont know" due to scans being missed sometimes.
2 - the case sensitivity caused it to fail even though the scanning would work. This lead to different test cases.
3 - Due to different behaviour with certain BLE drivers the library might not work at all, work partially or work normally.

@vliedel has proposed the following improvements to the library:

  • Add getMode(), throw an error when there was no scan before timeout.
  • Add waitForMode(setup / normal / dfu), throw an error on timeout.
  • Make mac address case insensitive.
  • Make the lib work without setting keys.
  • Add getMode() when connected. (returns setup/dfu/normal/unknown enum)
  • Add Logger

This new API would cover all problems listed in this issue, as well as some others.

This issue is NOT solved at the moment and should not be closed until it is solved. We'd like to have the changes in the python library implemented within the next 2 weeks.

This issue will be closed upon the release of the new API addressing the issue.

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

No branches or pull requests

3 participants