Add acceleromter range and rate acccessors (in progress). #266

carlosperate · 2016-05-04T08:10:58Z

Didn't really get the chance last night, but I found some time this morning to play with this. I've encountered something a bit odd with my current implementation, so I'd though I'd create the PR for quick review (it is rather simple) in case I'm not setting something correctly.

When I set the range and rate it seems like it is heavily rounding up the final calculation. So to set a sampling frequency of 50hz, I need to set the value 45 or 46, anything higher sets it to 100hz. Similar thing happens with the range.

I'll see if I can replicate tonight the same behaviour using a simple Cpp program with just the DAL.

Resolves #264

carlosperate · 2016-05-04T08:11:56Z

docs/accelerometer.rst

+.. py:function:: set_range()
+
+    Set the accelerometer sensitivity range, in Hz.
+


Should probably add more info about what changing the range actually does.

Yes, include the possible values to set.

dpgeorge · 2016-05-04T08:56:37Z

When I set the range and rate it seems like it is heavily rounding up the final calculation.

I think the accelerometer can only do specific rates/periods: 1.25ms, 2.5ms, 5ms, 10ms, 20ms, 80ms, 160ms, 640ms.

Also it's restricted to 2, 4 or 8 g's.

carlosperate · 2016-05-04T09:04:23Z

Yes, I meant when trying to set an exact accepted value, so let's say 20ms/50Hz, in that case it seems to actually set it to 10ms/100Hz, I have to do microbit.accelerometer.set_rate(45) for it to actually resolve to 50Hz. Same thing with the g sensitivity.

I had a quick look at the DAL code, but I didn't have enough time before work to actually think through it to ensure it works as I expected. I assume that using the ::setPeriod(20) would result in setting that exact value, so I need to find out if somehow I'm sending the wrong value to the DAL api, or if that works slightly differently than what I expected.

carlosperate · 2016-05-04T11:07:05Z

source/microbit/microbitaccelerometer.cpp

+
+mp_obj_t microbit_accelerometer_set_range(mp_obj_t self_in, mp_int_t g) {
+    microbit_accelerometer_obj_t *self = (microbit_accelerometer_obj_t*)self_in;
+    return mp_obj_new_int(self->accelerometer->setRange(g));


Need to change this to return mp_const_none;

carlosperate · 2016-05-05T11:33:33Z

The using the DAL on a plain Cpp application sworks as expected on version 2. I tried also using version 1.4.15 and I've had some issues getting the program to create a microbit-combined.hex file. Had to convert my main() application to the old app_main(), and while it compiled fined, it only produced the microbit.hex file (and after loading that one nothing really happens). Is there any old documentation somewhere where I could check what exactly I might need to do?

carlosperate · 2016-05-07T22:32:58Z

Okay, so I was finally able to spend a bit more time on this. So I went back and fixed the issue I had building the C++ program using the 1.4.15 version of the DAL, and as expected, that works well.

I looked at the data being sent to the DAL accelerometer instance and noticed that it was basically the user input >> 1 | 1, so I started looking around and noticed that's how the mp objects for integers are formed. Looks like the function arguments in the line before were passed as mp_obj_t instead of mp_int_t

mp_obj_t microbit_accelerometer_set_rate(mp_obj_t self_in, mp_int_t hz)

Is that always the case? I saw a few cases around using mp_int_t and I just assumed the data would be formated as integers. Is there any information online that describes some or any of this stuff? Googling I only found a rather small entry in the micropython wiki about creating new modules.

Anyway, it is working now, however because we are doing integer arithmetic with the frequency conversion to a period value in miliseconds, which is then converted in the DAL to microseconds, we do lose some accuracy. For the rate values configured in the accelerometer, we will read them as:

const MMA8653SampleRateConfig MMA8653SampleRate[MMA8653_SAMPLE_RATES] = {
    {1250,      0x00},       // Read as 1000Hz, supposed to be 800Hz
    {2500,      0x08},       // Read as 500Hz, supposed to be 400Hz
    {5000,      0x10},       // Read as 200Hz, correct
    {10000,     0x18},       // Read as 100Hz, correct
    {20000,     0x20},       // Read as 50Hz, correct
    {80000,     0x28},       // Read as 12Hz, supposed to be 12.5Hz
    {160000,    0x30},       // Read as 6Hz, supposed to be 6.25Hz
    {640000,    0x38}        // Read as 1Hz, supposed to be 1.5625Hz
};

Apart from that, if the user tries to set the rate to 0Hz, it will be configure to the 1250 us / 800Hz value.

carlosperate · 2016-05-12T11:24:59Z

Apart from the extra info needed in the documentation, is the implementation correct based on what you had in mind with #264?

markshannon · 2016-05-13T03:18:02Z

OOI why is the API in terms of rate, rather than period?
Given that only a fixed set of rate/periods are available would it make sense to make a set of symbolic constants available?

carlosperate · 2016-05-27T06:24:06Z

OOI why is the API in terms of rate, rather than period?

I was implementing the suggested API from #264 , but period in ms or us would be easier to pass through.

Given that only a fixed set of rate/periods are available would it make sense to make a set of symbolic constants available?

I'm not a big fan of having constants for these, but maybe we should return the configured value? So (using still rate) it would look like this:

> microbit.accelerometer.set_rate(10)
12

carlosperate · 2016-06-03T19:08:56Z

@dpgeorge any feedback on this?

dpgeorge · 2016-06-03T20:40:38Z

@carlosperate it's very clean.

But now that we have the discussion in #296 about setting/getting the I2C frequency, I realise that configuring the accelerometer is similar, and maybe it should use an init() method to set the rate and range?

carlosperate · 2016-06-03T22:18:54Z

Well, the accelerometer object gets constructed by the DAL, so it would be a bit odd to have an init method if we are able to successfully execute any of the other methods without calling it first, no?

I guess this is already the case for UART, SPI, and now I2C, but those are internal peripherals which, generally speaking, are a bit more advanced and in most cases have to be "set up" before use. From my perspective the display, buttons, accelerometer, and compass are more like "pre-packaged" components, for which we are facilitating their use, but are ready out of the box. Having a init function suggests that they need to go through a process of initialisation, that we are completely bypassing in most situations, and I would personally find that slightly confusing.

On the other hand having that init would continue a general convention, which could be good. So I don't mind that much either way.

Which way should we go?

dpgeorge · 2021-09-06T11:43:41Z

docs/accelerometer.rst

+
+    Get the accelerometer sensitivity range, in g's.
+
+.. py:function:: set_range()


needs to be set_range(value)

dpgeorge · 2021-09-06T11:44:10Z

docs/accelerometer.rst

+
+    Get the accelerometer samping rate, in Hz.
+
+.. py:function:: set_rate()


I think we should remove the get/set rate methods, there is no such functionality in MakeCode

To save memory we need to keep the additions to a minimum. The sampling rate is not exposed in MakeCode and has never been requested yet in MakeCode nor MicroPython. set_range is available in MakeCode and has been requested in MicroPython to be able to create equivalent programmes and lessons. get_range has not been requested, is not available in MakeCode, and there is not a lot of cases where it would be useful, so it's been also removed to save memory.

carlosperate · 2021-09-23T11:57:06Z

Rebased and updated to remove the rate methods, and in the end I also removed get_range to save memory.

dpgeorge · 2021-10-25T05:56:57Z

Squashed and merged in 3148254

carlosperate reviewed May 4, 2016
View reviewed changes

dpgeorge mentioned this pull request Jun 6, 2016

Add support for MicroBitI2C.set/get_frequency(). #296

Closed

carlosperate mentioned this pull request Jun 10, 2016

Setting parameters consistency #306

Open

dpgeorge mentioned this pull request Jul 22, 2016

Getting access to accelerometer setRange #327

Closed

microbit-mark mentioned this pull request Jun 8, 2020

Allow to configure range (and rate?) of accelerometer #264

Closed

dpgeorge reviewed Sep 6, 2021

View reviewed changes

carlosperate added 2 commits September 22, 2021 23:54

Add acceleromter range and rate acccessors.

83ee591

carlosperate force-pushed the acc_rate_range branch from c09ed10 to 216e1ad Compare September 23, 2021 11:53

dpgeorge closed this Oct 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add acceleromter range and rate acccessors (in progress). #266

Add acceleromter range and rate acccessors (in progress). #266

carlosperate commented May 4, 2016 •

edited

Loading

carlosperate May 4, 2016

dpgeorge Sep 6, 2021

dpgeorge commented May 4, 2016

carlosperate commented May 4, 2016 •

edited

Loading

carlosperate May 4, 2016

carlosperate commented May 5, 2016

carlosperate commented May 7, 2016

carlosperate commented May 12, 2016

markshannon commented May 13, 2016

carlosperate commented May 27, 2016

carlosperate commented Jun 3, 2016

dpgeorge commented Jun 3, 2016

carlosperate commented Jun 3, 2016

dpgeorge Sep 6, 2021

dpgeorge Sep 6, 2021

carlosperate commented Sep 23, 2021 •

edited

Loading

dpgeorge commented Oct 25, 2021

		.. py:function:: set_range()

		Set the accelerometer sensitivity range, in Hz.


		Get the accelerometer sensitivity range, in g's.

		.. py:function:: set_range()


		Get the accelerometer samping rate, in Hz.

		.. py:function:: set_rate()

Add acceleromter range and rate acccessors (in progress). #266

Add acceleromter range and rate acccessors (in progress). #266

Conversation

carlosperate commented May 4, 2016 • edited Loading

carlosperate May 4, 2016

Choose a reason for hiding this comment

dpgeorge Sep 6, 2021

Choose a reason for hiding this comment

dpgeorge commented May 4, 2016

carlosperate commented May 4, 2016 • edited Loading

carlosperate May 4, 2016

Choose a reason for hiding this comment

carlosperate commented May 5, 2016

carlosperate commented May 7, 2016

carlosperate commented May 12, 2016

markshannon commented May 13, 2016

carlosperate commented May 27, 2016

carlosperate commented Jun 3, 2016

dpgeorge commented Jun 3, 2016

carlosperate commented Jun 3, 2016

dpgeorge Sep 6, 2021

Choose a reason for hiding this comment

dpgeorge Sep 6, 2021

Choose a reason for hiding this comment

carlosperate commented Sep 23, 2021 • edited Loading

dpgeorge commented Oct 25, 2021

carlosperate commented May 4, 2016 •

edited

Loading

carlosperate commented May 4, 2016 •

edited

Loading

carlosperate commented Sep 23, 2021 •

edited

Loading