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

Sending float values from Arduino Nano 33 BLE doesn't seem to work #6

Closed
armsp opened this issue Oct 7, 2019 · 12 comments
Closed

Sending float values from Arduino Nano 33 BLE doesn't seem to work #6

armsp opened this issue Oct 7, 2019 · 12 comments

Comments

@armsp
Copy link

armsp commented Oct 7, 2019

I got this wonderful library working with the new Arduino Nano 33 BLE. However sending float values seems to have an issue. For example these are the float values as reported in my serial montior and the corresponding float values received by p5.ble.js

Serial Monitor p5.ble.js received value
1.22 5.933522291385966e-39
1.16 1.0338756247714607e-38
0.92 8.854321548098327e-39
0.73 1.177619700203129e-38
1.28 1.528288335057325e-39
1.46 1.1822114750110285e-38
6.53 2.321725105554387e-38

I am using the ArduinoBLE library. And sending float values using BLEFloatCharacteristic and receiving them in p5.ble.js using myBLE.read(myCharacteristic, 'float32', gotValue);

@armsp
Copy link
Author

armsp commented Oct 7, 2019

So it looks like the corresponding values are indeed correlated according to this reply. What we need to figure out is how to swap the endianness of the data we receive.

@armsp
Copy link
Author

armsp commented Oct 8, 2019

Making the following changes in the code makes it work -

case 'float32':
      result = data.getFloat32(0, true); //'littleEndian' works too
      break;

@armsp
Copy link
Author

armsp commented Oct 9, 2019

I think @tigoe might make a pull request to solve the same. Feel free to close this now or later once the PR is merged.

@tigoe
Copy link
Collaborator

tigoe commented Oct 9, 2019

Haven't made a PR yet, but @armsp is correct in that the issue is that the current API doesn't give you a way to set the endianness of your data types, even though the functions it uses (e.g. getFloat32()) do. So it's more of a feature request, adding the ability to set endianness in the API.

This is actually the same issue as #4, which I originally thought was about float64, because I didn't pay enough attention.

@armsp
Copy link
Author

armsp commented Oct 9, 2019

@tigoe let me know if you need any assistance. I would be glad to contribute.

@tigoe
Copy link
Collaborator

tigoe commented Oct 9, 2019

If you want to make a PR that adds the ability to set endianness on all the functions in parseData.js, that would be helpful. My feeling would be that maybe it could be an optional parameter in startNotifications() around line 83 and following of p5.ble.js? I would defer to @yining1023 on whether that's the best place, or if there's somewhere better, though.

@armsp
Copy link
Author

armsp commented Oct 10, 2019

@yining1023 thanks for your wonderful work. Just wanted to check with you on @tigoe 's suggestions. You can take a look at this issue (the title is bit of a misnomer)and #4 to gain some insight into the discussion. Please do let me know what approach you'd like to prefer and I'd try my best to come up with a decent PR to close this.

@yining1023
Copy link
Member

yining1023 commented Oct 10, 2019

Hi @armsp, Thank you so much for reporting and solving the issue!

It would be amazing if you could add a PR to allow users to set the endianness of the data types when they are calling .read() or .startNotifications(). I think by default, the API will do data.getFloat32(0, true);, it seems like this use case is much more common. And if people specify it otherwise:

myble.read(myCharacteristic, 'float32', 'bigEndian', gotValue);
myble. startNotifications(myCharacteristic, handleNotifications, 'float32', 'bigEndian');

then in the parseData.js, we will do data.getFloat32(0); But this approach requires more changes to the API. If you would like to do a quick fix, it will also be super helpful to make a quick PR that updates the parseData.js to

data.getFloat32(0, true);
data.getFloat64(0, true);

so it uses littleEndian by default, and later we can add the feature to allow users to choose either littleEndian or bigEndian.

Thank you for your contribution! please let me know if you have any questions.

@armsp
Copy link
Author

armsp commented Oct 10, 2019

@yining1023 okay then, I will make a quick PR by changing the getFloatXX function and then later on we can modify the whole thing as you mentioned by keeping a default and being able to pass 'bigEndian'.

@yining1023
Copy link
Member

Awesome! Thank you!

yining1023 added a commit that referenced this issue Oct 10, 2019
Quick fix for issue #6 float data little endian
@yining1023
Copy link
Member

The changes are in p5.ble.js@0.0.5 now! Please let me know if it works for you.

I also added support for big-endian: #4 (comment)

@armsp
Copy link
Author

armsp commented Oct 15, 2019

@yining1023 I tested the code with the latest release. It works as expected. Thanks a lot for the support.

@armsp armsp closed this as completed Oct 15, 2019
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