Nodemcu-firmware: Hardware timer (platform_hw_timer_*) causing freeze

[vsky279]

I am building a module to radio control Somfy blinds via 433Mhz RF transmitter. A sequence of 0 and 1 with different delays needs to be transmitted as illustrated on the chart:

There are implementations for Arduino using delay functions but on ESP8266 I need to go asynchronous. That is why I have decided to use the hw_timer to achieve precise timing in the order of microseconds.

Actual behavior

I am indeed able to control blinds but after sending few commands the ESP8266 freezes and then goes restart triggered by hw watchdog (rst cause: 4, boot mode:(3,0)).

Test code

The minimum code to reproduce the issue is

modules\somfy.c

#include "os_type.h"
#include "osapi.h"

#include "module.h"
#include "lauxlib.h"
#include "platform.h"
#include "hw_timer.h"
#include "user_interface.h"

#define SYMBOL 640 // symbol width in microseconds

#define TIMER_OWNER ((os_param_t) 's')

static uint32_t   delay[10] = {9415, 89565, 4*SYMBOL, 4*SYMBOL, 4*SYMBOL, 4550, SYMBOL, SYMBOL, SYMBOL, 30415}; // 143 ms total
static uint8_t    signalindex;

static void ICACHE_RAM_ATTR sendCommand(os_param_t p) {
    (void) p;
    //NODE_DBG("%d\n", signalindex);
    signalindex++;
    if (signalindex<10) {
        platform_hw_timer_arm_us(TIMER_OWNER, delay[signalindex-1]);
    } else {
        platform_hw_timer_close(TIMER_OWNER);
    }
}

static int somfy_lua_sendcommand(lua_State* L) {
    if (!platform_hw_timer_init(TIMER_OWNER, NMI_SOURCE, TRUE)) {
        // Failed to init the timer
        luaL_error(L, "Unable to initialize timer");
    }
    NODE_DBG("Triggering hw timer (%d)\n",  system_get_time());
    platform_hw_timer_set_func(TIMER_OWNER, sendCommand, 0);
    signalindex=0;
    sendCommand(0);
    return 0;
}

static const LUA_REG_TYPE somfy_map[] = {
    { LSTRKEY( "sendcommand" ), LFUNCVAL(somfy_lua_sendcommand)},
    { LNILKEY, LNILVAL}
};

NODEMCU_MODULE(SOMFY, "somfy", somfy_map, NULL);

and the Lua code to trigger it

tmr.alarm(0, 250, tmr.ALARM_AUTO, function() somfy.sendcommand() end)

The Lua code freezes and crashes in less than 1 minute. The sendcommand calls platform_hw_timer_close() in less than 150ms so there should be no issue that the function is called when the timer is still running.

Am I doing something which is not compliant with specifications?

NodeMCU version

I am using dev branch, version up to 56b4a3e commit.
Modules included: file, gpio, net, node, tmr, uart, wifi.

Hardware

ESP-01. No need to connect the RF transmitter to reproduce the issue.

[devsaurus]

Interesting hardware project :smiley:

There are a couple of potential pitfalls in your code that are worth checking:

• All ISR resources have to be in RAM. Especially delay could be a problem in case the toolchain mapped it to rodata in flash. Check app/mapfile.
• I experienced random failures with NMI_SOURCE in the pcm driver. Better rule out system issues by using FRC1_SOURCE in the first place. It's probably accurate enough.
• IIRC there's no need to re-arm the hw_timer in the ISR. Edit: It will be interesting to learn about the implications of changing the reload value in the ISR. When will it become effective - immediately for the current shot or the one after that?

[vsky279]

Thanks for your great advice.

• Putting delay array in RAM solved the issue. For this I use c_malloc and c_memcpy. I am not sure if there is more efficient way.
• Changing the source to FRC1_SOURCE seem to solve residual 1% of failures. The precision is not crucial. Somfy receivers are tolerant to timing precision.
• I need to re-arm as the time interval between ISR need to change in different steps of transmitting the message. I don't know how to change reload value in the ISR. It won't be much different to RTC_REG_WRITE(FRC1_LOAD_ADDRESS, ticks); in the platform_hw_timer_arm_ticks function. No?
• I use platform_hw_timer_arm_ticks now. No need to convert from us to ticks every time the ISR is called.

Once the development is finalized I will open another issue to discuss if there is an interest to merge the somfy module. I am not sure the audience will be large enough so at least there will be a reference here for those who are interested. :smiley:

BTW I love your pcm module.

[marcelstoer]

Lukas, kudos for sticking to our new issue template 😃

[devsaurus]

For this I use c_malloc and c_memcpy. I am not sure if there is more efficient way.

We had this topic recently for the ws2812 and solved it by mapping the array manually. But I guess that it will be mapped to RAM automatically once your code starts to actually modify the contents. If you overwrite one entry from somewhere in the code (with a different value) then the toolchain should be forced to do all init and allocation stuff for you. Theoretically, can't try that right now.

I need to re-arm as the time interval between ISR need to change in different steps

Yeah, I figured that _after_ posting :blush: I updated with edit because it's really not clear to me how the hardware reacts when you set a new timer value while the timer was already reloaded. Maybe my model is incorrect:

1. Timer expires
2. Interrupt triggered & timer restarted
3. ISR executes and changes timeout value
4. Does the timer run on the old value which was effective when it restarted or will it anticipate the new?

Thanks for the cool tweet :wink: pcm is soon due to be merged.

[vsky279]

For this I use c_malloc and c_memcpy. I am not sure if there is more efficient way.

We had this topic recently for the ws2812 and solved it by mapping the array manually. But I guess that it will be mapped to RAM automatically once your code starts to actually modify the contents. If you overwrite one entry from somewhere in the code (with a different value) then the toolchain should be forced to do all init and allocation stuff for you. Theoretically, can't try that right now.

As the array is truly constant I use the same trick with mapping the array manually. It simplifies the code and eliminates risk of memory leak.

I need to re-arm as the time interval between ISR need to change in different steps

Yeah, I figured that after posting :blush: I updated with edit because it's really not clear to me how the hardware reacts when you set a new timer value while the timer was already reloaded. Maybe my model is incorrect:

1. Timer expires
2. Interrupt triggered & timer restarted
3. ISR executes and changes timeout value
4. Does the timer run on the old value which was effective when it restarted or will it anticipate the new?

I am not sure this will answer your question. The following code will print the second timestamp 10100 μs (+ ~43 μs) later after the first timestamp, then in 20100 μs, etc.

static void ICACHE_RAM_ATTR sendCommand(os_param_t p) {
    (void) p;
    NODE_DBG("%d\n",  system_get_time());
    delayMicroseconds(100);
    signalindex++;
    platform_hw_timer_arm_ticks(TIMER_OWNER, signalindex*US_TO_RTC_TIMER_TICKS(10000));
}

[devsaurus]

Well, if your solution works then fine :smiley:

Once the development is finalized I will open another issue to discuss if there is an interest to merge the somfy module.

Your use case is related to the existing gpio.serout() functionality, there's also a pending PR #1327 to fix some shortcomings. Maybe check this along the way as well. However, Terry's statements in https://github.com/nodemcu/nodemcu-firmware/issues/874#issuecomment-175205333 show that the current implementation is not free from collisions with random ISRs and you would have to expect additional delay thereof.
With this backgound your approach could be regarded as an ISR-based variant of gpio.serout().

[oyooyo]

With this backgound your approach could be regarded as an ISR-based variant of gpio.serout().

I thought the same when I saw this issue. While I have no personal interest in a somfy module, I'm very interested in a proper, asynchronous version of gpio.serout() using interrupts instead of busy-waiting.
So I would very much welcome if the code made its way into a function like gpio.seroutasync() or so that could then be used by multiple one-way communication protocols (for example using 433MHz or IR transmitters).

[vsky279]

I did not know about the gpio.serout() function but anyway it would be very hard to use it for Somfy driver purpose.

I have drafted the suggested gpio.seroutasync() function - see please see this commit in my branch. The implementation is based on the gpio.serout() function. I have tested it with LED and a speaker, not an oscilloscope, and it seems to work fine.

Although I have some doubts whether I do thing the right way. For example I am not sure it is correct to reference the lua_State* L after the function has exited. Also I am not sure what happens if the function is called while the transmission is still ongoing - probably a memory leak(?).

Feel free to comment and/or add PRs. I am a newbie so I let gurus comment whether this implementation of gpio.seroutasync() could be eventually merged.

It would make probably sense to add a Lua callback when the transmission is done. In the Somfy driver I am using a sw timer for this purpose (I think I should not call Lua from the ISR).

[oyooyo]

Thanks a lot @vsky279 , I'm quite excited for this! I think I will have a look at this tomorrow.

It would make probably sense to add a Lua callback when the transmission is done.

Yep, I believe this is pretty much a necessity.

[vsky279]

Latest commit is adding the callback feature.

[oyooyo]

I just connected my scope and tested gpio.seroutasync(). Everything seems to work just fine, with the exception of one tiny remaining issue:
It toggles one more additional time at the end.

For example, if the state of the specified GPIO pin is low before gpio.seroutasync(), and one calls
gpio.seroutasync(x, 1, {400, 600}, 1, ...), the GPIO pin is left in the "high" state when it finishes.

I guess this behaviour is strictly speaking not a bug, since the documentation of gpio.serout() does not explicitly mention how gpio.serout() should behave in this respect, but I personally prefer the following logic: The number of values in delay_times should be even, and in this case (if there is an even number of delay_times) the state of the GPIO pin should be left in the opposite state of start_level when gpio.serout(async) finishes.

Apart from that, I'm very pleased with your implementation! Since gpio.serout() is problematic due to being synchronous anyway, apparently never actually worked, and my PR for fixing gpio.serout() still hasn't been merged, it might be better to drop the current, synchronous version of gpio.serout() altogether and replace it with your asynchronous implementation.

[devsaurus]

@vsky279 regarding your question / implementation of the finished callback:
I never saw your approach before to arm a timer from ISR for callbacks. Can't assess whether it's ok in interrupt context. For the pcm module I used task_post_*() as mentioned in the Extension Developer FAQ.
Examples can be found in gpio and pcm.

@oyooyo I'm not sure whether we should drop the synchronous gpio.serout() completely. The hw_timer based asynchronous solution comes with constraints regarding the step time. IIRC Espressif specifies a minimum reload time of ~50 µs. We might end up with two flavors targeted for different use cases:

• synchronous operation with sub-50 µs resolution, restricted to max. overall duration
• asynchronous operation with less granularity but virtually unrestricted duration

Am I missing something here?

Both flavors could be further collapsed into

gpio.serout(pin, start_level, delay_times[, [repeat_num][, callback])

Setting or omitting a callback would switch between the asynchronous and synchronous variant respectively. (repeat_num could be made a mandatory parameter for simplicity)

What do others think?

[oyooyo]

@devsaurus Good point about the sub 50µs resolution, that sounds plausible.
And auto-switching between a synchronous and an asynchronous version indeed seems like the perfect solution. The biggest part of the synchronous implementation of gpio.serout consists of checking the arguments and copying the delay_times into a C array. Most of that happens in the asynchronous version as well, so merging both versions should also reduce code size.

[vsky279]

I have merged gpio.serout and gpio.seroutasync as suggested. I like the idea. It's backwards compatible and does not increase the code size much while both versions are still available. I have added one feature - if callback parameter is numeric the gpio goes async but no callback is triggered.

@oyooyo small modification and gpio.serout behaves the way you describe. The modification is that the last time ISR is called the GPIO state is not inverted.

@devsaurus I have fixed also the way the callback was triggered. The way it was implemented was funny and kind of a workaround. I did not know about this straightforward way.

Latest commit.

[oyooyo]

@vsky279 I tried your latest version and I'm perfectly satisfied. Well done!

[devsaurus]

@vsky279 I second @oyooyo - Guess the code is ready for a PR :wink:
Please squash the commits upfront as this will present the effective changes and simplifies review.

[devyte]

@vsky279 will you be posting the final version of your somfy controller somewhere? I've been messing around with 433MHz things recently, and it would be interesting to have a wifi-433MHz bridge. If you add a receiver, it would make the bridge bidirectional, which would be awesome.

On a side note, I also have somfy motors with 433MHz receivers, but I've been sort of going a different route due to security. If you'd like to know more, just let me know.

[vsky279]

@devyte Latest version of the Somfy driver is available here: https://github.com/vsky279/nodemcu-firmware/blob/somfy/app/modules/somfy.c.
I will fix the issues that were pointed out in the PR for gpio.serout and I will open an issue on new feature request. However I don't think it will be accepted as the number of potential users is very likely close to zero :-)

At the same time I don't think the Somfy driver will help you anyhow to implement the 433MHz bridge. The driver just encodes the signal so it is understood by the Somfy controller (see the image in the first post). But it may inspire you...

I'm definitely interested in your way to control Somfy motors. My blinds application is not susceptible to security issues. Anyway the level of security remains the same as with standard Somfy controller...

[devsaurus]

The PR was merged, guess we can close this now.