Skip to content

CurieBLE: StandardFirmata BLE does not work. #377

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

Closed
sandeepmistry opened this issue Dec 29, 2016 · 26 comments
Closed

CurieBLE: StandardFirmata BLE does not work. #377

sandeepmistry opened this issue Dec 29, 2016 · 26 comments
Assignees

Comments

@sandeepmistry
Copy link
Contributor

I'm testing the latest master release with PR firmata/arduino#335.

However, I think the board is crashing - I don't see the 101 advertising using BLE, and the next time I try to load a sketch I have to press the master reset button.

I would expect the sketch to work with @soundanalogous's Node.js test app: https://gist.github.com/soundanalogous/927360b797574ed50e27

@sgbihu
Copy link
Contributor

sgbihu commented Jan 5, 2017

Hi @sandeepmistry ,

I has shoot the root cause of this issue. The Firmata try to add profile before HW initialized. That makes the program crashed. So I move some process from BLEStream constructor to begin and that makes the program works. I can discover the profile with another 101 board.

The fix commit ID is 10c482f. My github is https://github.com/sgbihu/corelibs-arduino101.

I didn't test with https://gist.github.com/soundanalogous/927360b797574ed50e27. Because I don't have access to it. And I didn't know how to set the environment. So can you help test it?

@SidLeung
Copy link
Contributor

SidLeung commented Jan 6, 2017

This issue is logged as Jira #791.

@SidLeung
Copy link
Contributor

SidLeung commented Jan 6, 2017

This issue is logged as Jira 791. Solution from Liang under internal review, testing. Will update status here upon completion.

sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Jan 11, 2017
…rduino#377

1. The BLEStream add profile when boot but the HW doesn't be initialized and
    makes APP crash.
2. Change CommunitySketches/StandardFirmataBLE/BLEStream.h.
    Move the add profile at begin after HW complete initial process.
sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Jan 11, 2017
…rduino#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change CommunitySketches/StandardFirmataBLE/BLEStream.h.
    Move the add profile at begin after HW complete initial process.
@sandeepmistry
Copy link
Contributor Author

Hi @sgbihu,

There are a few concerns about the items you proposed in #377 (comment):

  1. It requires a change to the upstream Firmata library. I would prefer we explorer a change to the CurieBLE library itself as other user sketches might follow the same pattern.

  2. If a BLEStream::begin(...), BLEStream::end(), BLEStream::begin(...) sequence is followed the attributes will be added again in begin.

sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Jan 12, 2017
…rduino#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change CommunitySketches/StandardFirmataBLE/BLEStream.h.
    Move the add profile at begin after HW complete initial process.
@SidLeung
Copy link
Contributor

Hi, @sandeepmistry

Agree with you that this is not the solution.

The issue actually exposes a problem with the robustness of the BLE library initialization process and we are going to address it (instead of making changes to the 3rd party code).

The cause of the system crash can be traced back to the instantiation of a global object using a constructor that makes BLE API calls at the time when the BLE library has not been initialized.

The instantiation of global object is (determined at compile time) done as part of the system power cylce/reset initialization. Object instantiation is performed by the Class Constructor. The BLE objects, however, are instantiated at run time (in order to reduce heap space and BSS consumption for sketches that do not use BLE), via the, begin(), API call in a sketch. Thus, if BLE API's are called in a constructor of a global object, these calls are made prior to proper BLE initialization.

We are working a solution to overcome this issue.

sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Jan 13, 2017
…rduino#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change the BLE initial process not depend on HW. Buffer the
    attributes if HW not initialized.
3. Changed files
    libraries/CurieBLE/src/BLEDevice.h - expose constructor
    libraries/CurieBLE/src/BLEPeripheral.cpp - not call HW init function
    libraries/CurieBLE/src/internal/BLEDeviceManager.cpp - change init order
    libraries/CurieBLE/src/internal/BLEProfileManager.cpp - change constructor
@sgbihu
Copy link
Contributor

sgbihu commented Jan 13, 2017

Hi @SidLeung and @sandeepmistry ,

I implement a solution. Please help review again.

Thanks!
-Gaoliang

sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Jan 16, 2017
…rduino#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change CommunitySketches/StandardFirmataBLE/BLEStream.h.
    Move the add profile at begin after HW complete initial process.
@noelpaz
Copy link
Contributor

noelpaz commented Jan 24, 2017

Currently testing this. Do we have to test using the node js app above/

@sandeepmistry
Copy link
Contributor Author

@noelpaz if possible that would be great, if you can't get it setup for whatever reason I can test it out later.

Once things work, we can ping firmata/arduino#335 to be merged.

SidLeung pushed a commit to SidLeung/corelibs-arduino101 that referenced this issue Jan 26, 2017
…#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change the BLE initial process not depend on HW. Buffer the
    attributes if HW not initialized.
3. Changed files
    libraries/CurieBLE/src/BLEDevice.h - expose constructor
    libraries/CurieBLE/src/BLEPeripheral.cpp - not call HW init function
    libraries/CurieBLE/src/internal/BLEDeviceManager.cpp - change init order
    libraries/CurieBLE/src/internal/BLEProfileManager.cpp - change constructor
eriknyquist pushed a commit that referenced this issue Feb 8, 2017
1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change the BLE initial process not depend on HW. Buffer the
    attributes if HW not initialized.
3. Changed files
    libraries/CurieBLE/src/BLEDevice.h - expose constructor
    libraries/CurieBLE/src/BLEPeripheral.cpp - not call HW init function
    libraries/CurieBLE/src/internal/BLEDeviceManager.cpp - change init order
    libraries/CurieBLE/src/internal/BLEProfileManager.cpp - change constructor
@SidLeung
Copy link
Contributor

SidLeung commented Feb 8, 2017

Code mods passed code review, system testing, and emerged to main trunk. Will be available in the next nightly build.

SidLeung pushed a commit to SidLeung/corelibs-arduino101 that referenced this issue Feb 9, 2017
…#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change the BLE initial process not depend on HW. Buffer the
    attributes if HW not initialized.
3. Changed files
    libraries/CurieBLE/src/BLEDevice.h - expose constructor
    libraries/CurieBLE/src/BLEPeripheral.cpp - not call HW init function
    libraries/CurieBLE/src/internal/BLEDeviceManager.cpp - change init order
    libraries/CurieBLE/src/internal/BLEProfileManager.cpp - change constructor
sgbihu added a commit to sgbihu/corelibs-arduino101 that referenced this issue Feb 13, 2017
…rduino#377

1. The BLEStream object adds profile when boot but the HW doesn't be
    initialized and makes APP crash.
2. Change CommunitySketches/StandardFirmataBLE/BLEStream.h.
    Move the add profile at begin after HW complete initial process.
@sandeepmistry
Copy link
Contributor Author

There's still something up with this, if I test with the instructions in #377 (comment) it doesn't work connecting from my Mac using Node.js

Debug wise with #define BLE_SERIAL_DEBUG enabled in BLEStream.h I just see:

BLEStream::received(F9)
BLEStream::received(F079F7)

@sgbihu
Copy link
Contributor

sgbihu commented Feb 18, 2017

Hi @sandeep,

I don't have access to get the nodejs example, can you share it to me?

Thanks!

@sandeepmistry
Copy link
Contributor Author

@sgbihu it's the Github gist link in the first comment: #377 (comment)

@kitsunami
Copy link

@SidLeung Can you evaluate if this is multiple tickets, and open new issues as needed?

@kitsunami kitsunami modified the milestones: Elnath, Deneb Feb 24, 2017
@noelpaz
Copy link
Contributor

noelpaz commented Mar 2, 2017

I tested using the instructiosn from sandeep and using ble-firmata-test.js and it looks like this is working. Here is the log from the mode.js application.

BLE-firmata.txt
Corelibs_Issue_377.zip

Steps:

  • I pulled the PR for the changes to firmata library and put it in the location in teh Arduino 1.8 IDE folder structure
  • I created a JSON based on PR-411 (this has internal links so I am including the zip of the corelibs)
  • I downloaded bleno and noble
  • I got the ble-firmata-test.js
  • Installed node
  • Installed npm
  • Installed apm
  • then from npm installed ble-serial and firmata
  • Wired as per instructions
  • ran the node.js script : DEBUG=ble-serial node ble-firmata-test
  • captured the logs

@noelpaz
Copy link
Contributor

noelpaz commented Mar 2, 2017

I forgot to make the edits to utility/BLEStream.h so I did that and got the same result. Here is the log
BLE-firmata_with_DEBUD_BLE-Strem.log.txt

@noelpaz noelpaz assigned sandeepmistry and unassigned noelpaz and SidLeung Mar 2, 2017
@soundanalogous
Copy link

It looks like it crashes silently right here:

ble-serial writing +1ms <Buffer f0 79 f7>

If successful you would see something like the following printed to the console shortly afterwards:

StandardFirmataBLE.ino-2.5

per these lines

I'll take a look at it this weekend to see if I can determine what the issue may be. The node.js script can also be fussy at times. It doesn't work consistently.

@soundanalogous
Copy link

A reply is never received from the board via Firmata.

@sgbihu
Copy link
Contributor

sgbihu commented Mar 2, 2017

Can you post some serial command and expected response here? In this way, I can use other device to simulate the process.

@SidLeung
Copy link
Contributor

SidLeung commented Mar 2, 2017

@soundanalogous ,

Per Sandeep's observation, he noted above that,

Debug wise with #define BLE_SERIAL_DEBUG enabled in BLEStream.h I just see:

BLEStream::received(F9)
BLEStream::received(F079F7)

Is the debug flag needed to be set in order to determine if there is any reply?

@soundanalogous
Copy link

The debug flag is not necessary.

@noelpaz
Copy link
Contributor

noelpaz commented Mar 2, 2017

Thanks. We were under the impression that if you see only these lines

BLEStream::received(F9)
BLEStream::received(F079F7)

That it has failed.

I have 2 logs one with the debug on set on BLEStream.h

Sandeep said that the BLE is not advertising as the symptom. Now with this new script what is actually not working? Is it our library or the script which is suppose to be unstable at times or the firmata library.? Protocol wise what transaction should the 101 be sending out.

We will run this through a protocol analyzer.

Also when I stop the node JS application, the Arduino 101 is scannable and connectable again. So BLE did not crash as has been implied.

@noelpaz
Copy link
Contributor

noelpaz commented Mar 2, 2017

@soundanalogous . I did not wire the components last night as per the instructions in the script but I will do so this morning and run it again. All I wanted to test was if your node js script can connect to the 101 and it did so and explored the peripheral which I thought was that was needed. Could it be the reason why? Also How should we be running the potentiometer and what is the spec for that potentiometer , taper and impedance will be helpful to reproduce these results.

@sandeepmistry
Copy link
Contributor Author

I'll try this out with the v1.0.7 release shortly and repeat back on the expected results.

@sandeepmistry
Copy link
Contributor Author

Here's what I get with v1.0.7

$ node ble-firmata-test.js 
StandardFirmataBLE.ino-2.5
606
pin 2 value: 0
pin 16 value: 1
448
375
332
310
297
290
287
283
282
281
282
281
278
289
293
295
297
pin 2 value: 1
289
284
283
282
280
289
295

Which lines up with the following statements in the JS:

  console.log(
    board.firmware.name + "-" +
    board.firmware.version.major + "." +
    board.firmware.version.minor
  );

  // ...


board.analogRead(0, function(val) {
    if (val !== lastVal) {
      console.log(val);
    }
    lastVal = val;
  });

  board.digitalRead(dBtnPin, function (val) {
    console.log("pin " + dBtnPin + " value: " + val);
  });

  board.digitalRead(aBtnPin, function (val) {
    console.log("pin " + aBtnPin + " value: " + val);
  });

// ...

@soundanalogous
Copy link

Yep that is the expected output :)

@sandeepmistry
Copy link
Contributor Author

This works for me with the Github master version of the core (d818a3a) now. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants