tijmenvangulik / ErgometerJS

Java script ergometer driver for concept2 performance monitor with BLE/USB .
Other
112 stars 35 forks source link

BLE driver issues #13

Closed AuspeXeu closed 4 years ago

AuspeXeu commented 4 years ago

I am having a lot of issues with the BLE driver it seems. After connecting I get this log message a lot NotSupportedError: GATT operation failed for unknown reason. After a while it will transmit partial data from the PM5. However not everything there should be and I also get tons of this logged. wrong csafe frame ending.

Any thoughts how to zero in on this bug?

It seems like this might be related: https://bugs.chromium.org/p/chromium/issues/detail?id=664863

tijmenvangulik commented 4 years ago

You have to give me some more information on what you are exactly doing (which example/platform/ways of connecting/hardware, etc..) Normally it is because of one of the internal drivers is not correctly installed.

Tijmen

On 15 Apr 2020, at 18:05, Chris Vaas notifications@github.com wrote:

I am having a lot of issues with the BLE driver it seems. After connecting I get this log message a lot NotSupportedError: GATT operation failed for unknown reason. After a while it will transmit partial data from the PM5. However not everything there should be and I also get tons of this logged. wrong csafe frame ending.

Any thoughts how to zero in on this bug?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NVEYG2VM3CP3JGECADRMXLOFANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

I am trying to use the BLE driver on the web. Hardware is my computer which has bluetooth and I am doing that inside chrome.

According to the link I attached in my first comment, it seems like you should serialize the calls to startNotifications.

tijmenvangulik commented 4 years ago

What kind of computer mac/pc , version os? version chrome?

On 15 Apr 2020, at 18:48, Chris Vaas notifications@github.com wrote:

I am trying to use the BLE driver on the web. Hardware is my computer which has bluetooth and I am doing that inside chrome.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614152833, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NWGGTOYEWBHKUPFMSLRMXQNBANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

Windows 10, chrome 81.

tijmenvangulik commented 4 years ago

If you start ergometer-space.org http://ergometer-space.org/ on your machine? Does it work? (uses the same version of ergometerjs as I last published )

On 15 Apr 2020, at 19:12, Chris Vaas notifications@github.com wrote:

Windows 10, latest chrome.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614165253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NUFH56ZV32AADCVTNTRMXTF5ANCNFSM4MIWP64Q.

tijmenvangulik commented 4 years ago

There are some requirements for using web blue tooth. For example if it is not local host you must use https.

On 15 Apr 2020, at 19:12, Chris Vaas notifications@github.com wrote:

Windows 10, latest chrome.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614165253, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NUFH56ZV32AADCVTNTRMXTF5ANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

It's localhost. Ergometer-space.org has the same issues it seems.

tijmenvangulik commented 4 years ago

if ergometer-space.org http://ergometer-space.org/ does not work, there must be some thing with windows or your ergometer. Do you have the latest version of windows 10 (the first versions of windows 10 had limited blue tooth drivers, chrome depends on the latest) and the PM5 monitor firmware (older version can be buggy).

On 15 Apr 2020, at 19:22, Chris Vaas notifications@github.com wrote:

It's localhost. Ergometer-space.org has the same issues it seems.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614170898, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NRUFC7GLM3BVXUUQRLRMXUMFANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

Well ... according to the link, in case you haven't had a chance to look at it. It can cause issues if the startNotifications commands are called in parallel instead of in sequence. And I checked your code, you're doing them all at once.

tijmenvangulik commented 4 years ago

I have seen that problem before, on android, On android the driver now schedules every thing after each other. This may be also a problem of the blue tooth hardware. In the utilities there is a routine promiseAllSync , in the past I used this to work around this problem, but it was not needed any more (and it slows down the init). I am interested if you replace the Promise.all with the promiseAllSync, if your problem is then fixed. Maybe we have to set it back

Tijmen

On 15 Apr 2020, at 21:24, Chris Vaas notifications@github.com wrote:

Well ... according to the link, in case you haven't had a chance to look at it. It can cause issues if the startNotifications commands are called in parallel instead of in sequence. And I checked your code, you're doing them all at once.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

AuspeXeu commented 4 years ago

Well, your promiseAllSync does not actually serialize the execution of the promises. they are just awaited in a different way. The execution is started upon new Promise which is before the promiseAllSync call.

tijmenvangulik commented 4 years ago

I am currently very busy with making the a team version of ergometer-space. Can you make a better solution?

Tijmen

On 15 Apr 2020, at 23:04, Chris Vaas notifications@github.com wrote:

Well, your promiseAllSync does not actually serialize the execution of the promises. they are just awaited in a different way. The execution is started upon new Promise which is before the promiseAllSync call.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614279024, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NW6JBGCN3BNJJ3EQ3DRMYOMVANCNFSM4MIWP64Q.

tijmenvangulik commented 4 years ago

I have just checked the function. As fare as I know, promises are created directly executed later. And code in the “then" is executed after the other promise. This is exactly what this function does do executes the next promise in the array using the “then”. So this should work or there is some thing else which is going wrong and we are searching in the wrong direction..

On 15 Apr 2020, at 23:04, Chris Vaas notifications@github.com wrote:

Well, your promiseAllSync does not actually serialize the execution of the promises. they are just awaited in a different way. The execution is started upon new Promise which is before the promiseAllSync call.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-614279024, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NW6JBGCN3BNJJ3EQ3DRMYOMVANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

This is not correct. The promise code starts executing immediately after the promise is instantiated. The then code simply means this is called after the promise finished. I have a small fix in my version right now which I'll be trying out today and see if it helps.

Edit: Just try the following code. By your logic, none of the code inside the promises should run, since we don't use any then.

for(let i = 0; i < 10; i++) {
  new Promise(() => console.log(i))
}
AuspeXeu commented 4 years ago

Yeah, so this still seems unresolved for me. I tried to run this on my Android phone. and am getting these weird GATT errors.

tijmenvangulik commented 4 years ago

Now you have these problems on two devices. Just to be sure do you also have these problems if you run ergometer-space on your phone. Ergometer space is used by many users without problem. I have only one report of a user which has problems on an old phone which he can not upgrade. If the same version of ergometer-space does not work on both the vices (pc and phone) then it is more likely that it has some thing todo with your PM5. What is the version info hardware/software of your PM5?.

On 20 Apr 2020, at 20:08, Chris Vaas notifications@github.com wrote:

Yeah, so this still seems unresolved for me. I tried to run this on my Android phone. and am getting these weird GATT errors.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-616722243, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NSN4TKL4GEUUFZHC7LRNSFRPANCNFSM4MIWP64Q.

AuspeXeu commented 4 years ago

It is a fairly old phone. You have the latest version from the git repo running on your erg space, no?

tijmenvangulik commented 4 years ago

It is a app which you can install from the app store

https://play.google.com/store/apps/details?id=org.tijmenvangulik.ergometerspace&hl=en https://play.google.com/store/apps/details?id=org.tijmenvangulik.ergometerspace&hl=en

On 20 Apr 2020, at 20:36, Chris Vaas notifications@github.com wrote:

It is a fairly old phone. You have the latest version from the git repo running on your erg space, no?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/issues/13#issuecomment-616735633, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NVCRYUFJDC5JA22BVDRNSIZTANCNFSM4MIWP64Q.

tijmenvangulik commented 4 years ago

I could for the first time repeat the connection problems. Hopefully I can now debug and fix this.

tijmenvangulik commented 4 years ago

I have published a fix for multi plex mode for android. You can turn on this mode and then it should compensate for limited blue tooth hard ware.

AuspeXeu commented 4 years ago

Thanks