[WIP] Feature/#2 accelerometer #53

sdorre · 2020-07-24T15:34:57Z

Import files from Bosch BMA4 library.

Prototypes that shows step counter and sensor temperature on screen

Fixes #48

TODO:

Implement interrupt features from sensor

sdorre · 2020-07-24T15:36:37Z

@endian-albin If you wanna try on your watch.
This version is the minimum implementation. There is no interrupts/power management and other special features.
But you can already show step count on screen.

drivers/sensor/bma421/bma421.c

endian-albin · 2020-07-24T23:20:49Z

hypnos/src/accelerometer.c

+	LOG_DBG("Accelerometer init: Done");
+}
+
+void accelerometer_show_health_data()


I wouldn't call it "health" data.

endian-albin · 2020-07-24T23:22:06Z

hypnos/src/event_handler.c

@@ -126,17 +127,21 @@ void button_pressed_isr(struct device *gpiobtn, struct gpio_callback *cb, uint32
 		bt_on();
 	}
 	gfx_update();
+	backlight_enable(true);


Yeah, that's probably better

On second thought, this appears to make the GUI feel less responsive. I prefer to have the screen light up fast, even if some data and graphics have not yet been updated. Please compare yourself by putting backlight_enable(true); before clock_increment_local_time();

endian-albin · 2020-07-25T10:04:27Z

I understand that this is a work-in-progress branch, but rather than restoring the state with 7baaf2a ("Remove mess from dts"), it's better to delete the commit that introduced the change in the first place.

endian-albin

It's really cool that you've made the accelerometer work this well. It needs to be cleaned up a bit, both to conform with the coding style and to make each commit meaningful (you can use interactive rebase to fix the latter). In addition, I'd like this to be interrupt driven before merging. Right now, the step counter gets disabled if you switch between BT mode on or off. I'd be happy to have a look again after you've worked on this a bit more.

drivers/sensor/bma421/bma421.c

endian-albin · 2020-07-25T15:25:52Z

drivers/sensor/bma421/bma421.c

-	} else {
+	switch((u16_t)chan) {
+	case SENSOR_CHAN_ACCEL_X:
+		// val->val1 = drv_data->accel_data.x;


Please use only C89-style comments (Zephyr style guidelines).

hypnos/src/event_handler.c

endian-albin · 2020-07-25T20:40:49Z

hypnos/src/accelerometer.c

+	}
+
+	snprintf(step_label_str, 32, "%d steps", step_count.val1);
+	snprintf(temperature_label_str, 32, "%d'C", temperature.val1);


The degrees symbol ° should be available in ASCII.

endian-albin · 2020-07-26T20:00:47Z

drivers/sensor/bma421/bma421_config.h is a binary blob, unfortunately, and can thus not be used in this project, especially not without a clearly defined origin and permission to use it. It would also be impossible to upstream it.

We discussed the accelerometer in the PineTime Matrix chat today and it was mentioned that a lot of things can be done without the so-called "configuration", but not step counting for example.

sdorre · 2020-07-27T07:38:40Z

drivers/sensor/bma421/bma421_config.h is a binary blob, unfortunately, and can thus not be used in this project, especially not without a clearly defined origin and permission to use it. It would also be impossible to upstream it.

We discussed the accelerometer in the PineTime Matrix chat today and it was mentioned that a lot of things can be done without the so-called "configuration", but not step counting for example.

Alright..
Boshc is not helping much on that topic but this blob is documented in the Sensor API Application note.
https://www.bosch-sensortec.com/media/boschsensortec/downloads/application_notes_1/bst-mas-an032-00.pdf

So I will write MY own blob, with credit on my name and give that to the project. All nicely documented.. Would that be okay to have that config file here then ?

endian-albin · 2020-07-27T08:44:36Z

I’m no lawyer, but if you could somehow, without violating copyright law, write a documented application that generates the blob, then it should be OK I think. Such a tool would also be beneficial to the other PineTime projects by the way. Using information that’s been made available without violating an NDA that you’ve signed yourself or stealing should be fine I believe. Thanks again for your efforts!

* Copy work from Daniel Thompson/Bosch Sensor Tech * Trying to use it as it is. Signed-off-by: Stephane Dorre <[email protected]>

* Seems to work now * Need to make sense of the values * Need to test step counter * implements #48 Signed-off-by: Stephane Dorre <[email protected]>

Signed-off-by: Stephane Dorre <[email protected]>

* Fetch data from the BMA421 driver * Update screen with new value Signed-off-by: Stephane Dorre <[email protected]>

sdorre · 2020-07-29T19:10:25Z

It's really cool that you've made the accelerometer work this well. It needs to be cleaned up a bit, both to conform with the coding style and to make each commit meaningful (you can use interactive rebase to fix the latter). In addition, I'd like this to be interrupt driven before merging. Right now, the step counter gets disabled if you switch between BT mode on or off. I'd be happy to have a look again after you've worked on this a bit more.

I don't get The step counter is disabled when we disable/enable BLE..

I. tried to generate a new config blob. Could you try on your side if that step counter still work for you as well ?

* take review comments into account Signed-off-by: Stephane Dorre <[email protected]>

Signed-off-by: Stephane Dorre <[email protected]>

endian-albin · 2020-07-29T22:09:20Z

It's really cool that you've made the accelerometer work this well. It needs to be cleaned up a bit, both to conform with the coding style and to make each commit meaningful (you can use interactive rebase to fix the latter). In addition, I'd like this to be interrupt driven before merging. Right now, the step counter gets disabled if you switch between BT mode on or off. I'd be happy to have a look again after you've worked on this a bit more.

I don't get The step counter is disabled when we disable/enable BLE..

The physical button toggles BLE advertising (and switches between two different threads. When I tested your code the last time, step counting always worked in the initial state, before BLE had been enabled by pressing the button, but it stopped incrementing the counter after pressing it.

I. tried to generate a new config blob. Could you try on your side if that step counter still work for you as well ?

No, it always stays at 0 unfortunately. The temperature value varies though. Does it work for you? What is your setup by the way?

endian-albin · 2020-07-29T22:28:49Z

I've now tested two commits a couple of times. Checking out, building and flashing "Add custom config blob" gives me a constant 0 steps. When I do the same with "Make code Zephyr compliant", i.e. the commit just before, steps are incremented, but I can't say if they are accurate or not. I tried pressing the physical button a few times and this didn't stop the step counter from working this time. I did a hard reset after programming each time (I've added a physical on/off switch to the watch).

sdorre · 2020-07-30T07:10:20Z

Okay. that is what I feared:
The first 70 bytes of the configuration are documented. Because those are all the parameters necessary to fine tune all the features.

The 5000+ following bytes would then be the initialization sequence of the internal ASIC and I guess provided by Bosch Sensor Tech directly..

Is that really impossible to use a blob ?
I mean, we could directly use it from the external repo where I found it ?
Or I could create a repo or a gist putting the right credit and license on it ?
(it was retro-engineering, so there is no NDA on that..)

If you think there is no solution to get that blob in this FW, I'll make you a simple accelerometer driver.. but I really don't like the idea of computing steps directly on the main uC.

endian-albin · 2020-07-30T09:47:56Z

Okay. that is what I feared:
The first 70 bytes of the configuration are documented. Because those are all the parameters necessary to fine tune all the features.

I see. Would these still be useful if we excluded the undocumented bytes?

The 5000+ following bytes would then be the initialization sequence of the internal ASIC and I guess provided by Bosch Sensor Tech directly..

Right, so it's a plain blob used to obscure how the hardware works.

Is that really impossible to use a blob ?
I mean, we could directly use it from the external repo where I found it ?
Or I could create a repo or a gist putting the right credit and license on it ?
(it was retro-engineering, so there is no NDA on that..)

There are two reasons why I don't want blobs in this project: 1) for philosophical reasons (I'm a Free Software supporter); 2) I want to work with upstream projects and eventually mainline all the out-of-tree drivers. I doubt blobs can be sent to upstream Zephyr.

If you think there is no solution to get that blob in this FW, I'll make you a simple accelerometer driver.. but I really don't like the idea of computing steps directly on the main uC.

How about dropping the steps counter feature for now and focus on other accelerometer features? It was mentioned in the Matrix chat that a lift-to-wake (light up the display) feature could be developed using timer interrupts every 100 ms or so that checks the XYZ values. Apparently, there is no explicit hardware support available for such a feature and so the implementation would need to be done this way anyway. Personally, I don't care about a step counter, but it would be really nice if the screen lights up by itself exactly when you want to look at it. I'm sure there are other features beside this that could be implemented without the blob. What do you think?

endian-albin · 2020-07-30T18:22:59Z

See also this open issue about binary blobs in the upstream Zephyr project.

endian-albin · 2020-08-03T22:54:23Z

More discussion took place on the PineTime chat tonight. The BMA421 blob used is apparently not under the BSD-3-Clause license but under a very restrictive license that we can certainly not redistribute. The author of the ATC1441 firmware also mentioned that he had rewritten the accelerometer library from scratch and integrated it directly with his Arduino-based project.

Signed-off-by: Stephane Dorre <[email protected]>

sdorre requested a review from endian-albin July 24, 2020 15:34

endian-albin reviewed Jul 24, 2020

View reviewed changes

drivers/sensor/bma421/bma421.c Outdated Show resolved Hide resolved

endian-albin reviewed Jul 24, 2020

View reviewed changes

endian-albin requested changes Jul 25, 2020

View reviewed changes

endian-albin reviewed Jul 25, 2020

View reviewed changes

sdorre added 5 commits July 29, 2020 20:54

BMA421 driver - First draft

2d520da

* Copy work from Daniel Thompson/Bosch Sensor Tech * Trying to use it as it is. Signed-off-by: Stephane Dorre <[email protected]>

Add accelerometer config

057cec8

* Seems to work now * Need to make sense of the values * Need to test step counter * implements #48 Signed-off-by: Stephane Dorre <[email protected]>

Move bma421 enum to a public header

65a8c5c

Signed-off-by: Stephane Dorre <[email protected]>

Extend GFX library to draw steps anad temperature

c2e5b87

Signed-off-by: Stephane Dorre <[email protected]>

Add accelerometer application module

d04b486

* Fetch data from the BMA421 driver * Update screen with new value Signed-off-by: Stephane Dorre <[email protected]>

sdorre force-pushed the feature/#2-Accelerometer branch from 7baaf2a to d04b486 Compare July 29, 2020 18:56

sdorre added 2 commits July 29, 2020 21:11

Make code Zephyr compliant

a472917

* take review comments into account Signed-off-by: Stephane Dorre <[email protected]>

Add custom config blob

8959a3d

Signed-off-by: Stephane Dorre <[email protected]>

sdorre added 4 commits August 6, 2020 09:27

Add configuration options for BMA421 driver

c9f3bb1

Signed-off-by: Stephane Dorre <[email protected]>

Add interrupt handling in BMA421 driver

1e3dcd3

Signed-off-by: Stephane Dorre <[email protected]>

Acceleration data fetched on data ready interrupt

4c1e13b

Signed-off-by: Stephane Dorre <[email protected]>

Extend Accelerometer features

7774349

Signed-off-by: Stephane Dorre <[email protected]>

sdorre added 2 commits August 9, 2020 20:21

Simple BMA421 driver featuring interrupt with Data Ready line.

c5a2148

Signed-off-by: Stephane Dorre <[email protected]>

Fix unnecessary changes

739edc1

Signed-off-by: Stephane Dorre <[email protected]>

sdorre closed this Aug 11, 2020

sdorre deleted the feature/#2-Accelerometer branch August 13, 2020 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Feature/#2 accelerometer #53

[WIP] Feature/#2 accelerometer #53

sdorre commented Jul 24, 2020

sdorre commented Jul 24, 2020

endian-albin Jul 24, 2020

sdorre Jul 29, 2020

endian-albin Jul 24, 2020 •

edited

Loading

endian-albin Jul 25, 2020

endian-albin commented Jul 25, 2020

endian-albin left a comment •

edited

Loading

endian-albin Jul 25, 2020

sdorre Jul 29, 2020

endian-albin Jul 25, 2020

endian-albin commented Jul 26, 2020 •

edited

Loading

sdorre commented Jul 27, 2020

endian-albin commented Jul 27, 2020 •

edited

Loading

sdorre commented Jul 29, 2020

endian-albin commented Jul 29, 2020 •

edited

Loading

endian-albin commented Jul 29, 2020 •

edited

Loading

sdorre commented Jul 30, 2020

endian-albin commented Jul 30, 2020 •

edited

Loading

endian-albin commented Jul 30, 2020

endian-albin commented Aug 3, 2020

[WIP] Feature/#2 accelerometer #53

[WIP] Feature/#2 accelerometer #53

Conversation

sdorre commented Jul 24, 2020

sdorre commented Jul 24, 2020

endian-albin Jul 24, 2020

Choose a reason for hiding this comment

sdorre Jul 29, 2020

Choose a reason for hiding this comment

endian-albin Jul 24, 2020 • edited Loading

Choose a reason for hiding this comment

endian-albin Jul 25, 2020

Choose a reason for hiding this comment

endian-albin commented Jul 25, 2020

endian-albin left a comment • edited Loading

Choose a reason for hiding this comment

endian-albin Jul 25, 2020

Choose a reason for hiding this comment

sdorre Jul 29, 2020

Choose a reason for hiding this comment

endian-albin Jul 25, 2020

Choose a reason for hiding this comment

endian-albin commented Jul 26, 2020 • edited Loading

sdorre commented Jul 27, 2020

endian-albin commented Jul 27, 2020 • edited Loading

sdorre commented Jul 29, 2020

endian-albin commented Jul 29, 2020 • edited Loading

endian-albin commented Jul 29, 2020 • edited Loading

sdorre commented Jul 30, 2020

endian-albin commented Jul 30, 2020 • edited Loading

endian-albin commented Jul 30, 2020

endian-albin commented Aug 3, 2020

endian-albin Jul 24, 2020 •

edited

Loading

endian-albin left a comment •

edited

Loading

endian-albin commented Jul 26, 2020 •

edited

Loading

endian-albin commented Jul 27, 2020 •

edited

Loading

endian-albin commented Jul 29, 2020 •

edited

Loading

endian-albin commented Jul 29, 2020 •

edited

Loading

endian-albin commented Jul 30, 2020 •

edited

Loading