fixed flash write and led signalling

[rananna]

@casainho , added the flash garbage collection to the wireless controller, and fixed a bug in the led signalling caused by calling a function twice. Bug report added to issue list.

[4var1]

Hey guys - sorry been a bit busy the last couple of days. On another training course and other stuff to do. Anything I need to look at? Assume you'd have shouted if i needed to fix anything :). @casainho did your Sunday ride go to plan - @rananna's remote all good? I haven't had a chance to test my code out on a ride yet - but will do so in the next day or so. I'm also building a new FSR over the next week and may well put a remote on that - haven't decided yet...

[rananna]

@4var1 , yes, I raised an issue in github. "leds will not light" basically If you call led_sequence_next two times in a row, the led sequence function stops working and leds will not light. This can happen with certain signalling options sometimes.

Also, I still need to use led_clear_queue() to stop a playing sequence as led_clear_queue_and_stop_current_sequence() leaves an led still lit.

[4var1]

@4var1 , yes, I raised an issue in github. "leds will not light" basically If you call led_sequence_next two times in a row, the led sequence function stops working and leds will not light. This can happen with certain signalling options sometimes.

Also, I still need to use led_clear_queue() to stop a playing sequence as led_clear_queue_and_stop_current_sequence() leaves an led still lit.

Hi - ok i'll take a look at the first issue. The 2nd - I'm still confused why you use that function - it's not exposed in the header - so please don't use it. Tell me what function I'm not exposing in the header you need and I'll see what I can do - but I'm confused why you need to use that? If I'm being obtuse please tell me :)

[4var1]

@4var1 , yes, I raised an issue in github. "leds will not light" basically If you call led_sequence_next two times in a row, the led sequence function stops working and leds will not light. This can happen with certain signalling options sometimes. Also, I still need to use led_clear_queue() to stop a playing sequence as led_clear_queue_and_stop_current_sequence() leaves an led still lit.

Hi - ok i'll take a look at the first issue. The 2nd - I'm still confused why you use that function - it's not exposed in the header - so please don't use it. Tell me what function I'm not exposing in the header you need and I'll see what I can do - but I'm confused why you need to use that? If I'm being obtuse please tell me :)

all of the play/now/next/until functions - and the cancel play until should cover the required situations I thought - please tell me if I've missed something

[4var1]

@rananna - sorry separate question - why are you still using nrf_delay in the remote code - wouldn't that be a source of the current spikes you mention? Why not replace all with your power efficient version?

[rananna]

As I said, the cancel play function led_clear_queue_and_stop_current_sequence left a lit led. led_clear_queue did not..... is there another cancel play function I should use?

[rananna]

in other words led_clear_queue_and_stop_current_sequence did NOT turn off the leds, but rather left them steady on....

[4var1]

in other words led_clear_queue_and_stop_current_sequence did NOT turn off the leds, but rather left them steady on....

Yes but I keep replying - why are you calling that function?? ;led_clear_queue_and_stop_current_sequence - it is not exposed in the header! You should not call it directly - there should be no need...

[4var1]

As I said, the cancel play function led_clear_queue_and_stop_current_sequence left a lit led. led_clear_queue did not..... is there another cancel play function I should use?

ah ok.... so what do you want to cancel, when? I only have a cancel for the repeated sequences - when do you need something different?

I assume you're 'extern' ing functions rather than just using what's in the header. That's part of the confusion - rather than digging into the internals - tell me what you need and I'll expose the required functionality in the header.

[4var1]

@rananna - seems we keep having half a conversation about this issue. You say that led_clear_queue_and_stop_current_sequence doesn't do what you expect - and I reply 'but that isn't in the header why are you using it - what do you need to do?' - can we try to conclude this rather than repeating the same again? :)

What are you trying to do - why do you need to stop and clear the queue and not play a sequence afterwards?

[rananna]

@4var1 , Yes apologies, I have been a little obtuse about this issue. To clarify - all is well now, with stopping a sequence. To stop a full queue, I simply needed to call led_sequence_play_now which plays one more blink and then stops. I was trying to stop without this extra blink by using led_clear_queue. On a separate topic I have replaced all my delays with the low power version. I simply wanted to test everything before doing a PR. The next PR will have the change.

Finally, the problem with calling led_sequence_play_next in sequence was causing NO further calls to led_sequence_play to work at all until the board was reset. However, I now can't repeat the issue! So ignore for now I guess.

[4var1]

@4var1 , Yes apologies, I have been a little obtuse about this issue. To clarify - all is well now, with stopping a sequence. To stop a full queue, I simply needed to call led_sequence_play_now which plays one more blink and then stops. I was trying to stop without this extra blink by using led_clear_queue. On a separate topic I have replaced all my delays with the low power version. I simply wanted to test everything before doing a PR. The next PR will have the change.

Finally, the problem with calling led_sequence_play_next in sequence was causing NO further calls to led_sequence_play to work at all until the board was reset. However, I now can't repeat the issue! So ignore for now I guess.

Ok no worries - i couldn't work out if I was being obtuse and you were annoyed! :)

Ok - if you do need to clear a queue I can expose a function to do that - but I wonder why that situation occurs? Why is the queue getting full of sequences? SHould you be using play_next or play_now rather than just play for the events that are filling up the queue perhaps?

You can also call play now with a sequence that is just an unlit LED for 0ms - that will do what you need without the extra blink - but again I wonder why the queue is getting full...

If you really do need to clear the queue - I'll expose a reset queue function if you'd prefer - save you having to use one of the above hacks..

On the other note - there was an issue where the 2nd sequence didn't play if you did two play_next with <50mS between the calls. But yes, the subsequent sequences would play fine - i didn't find that it disabled the led permanently.

But there was a bug - as lots of play next in <50mS should play the last sequence once - that wasn't happening.

So I put a dirty fix in - now when you play a sequence - after starting the app timer - it calls the ISR too to ensure that the sequence is registered, rather than waiting for the timer to fire to start. That gap was the cause of the lost sequence.

That has the side effect of speeding up the playing sequence by 50mS each time you call play_yada unfortunately.

I don't like that side-effect - will look at a better fix later this evening.

[rananna]

I have two cases that fill the buffer. A blinking red LED when brakes are held on, and a blinking GREEN led for walk mode. I wanted these to be always on to indicate to the user that each are active. power impact is minimal.

[4var1]

Ok you need to use play_xxx_until once - to start the sequence repeating - then cancel_play_until (i forget the exact wording) when the state is done? That does the hold_queue and release queue automatically for you. No need to stuff the buffer

[4var1]

led_sequence_play_now_until(LED_EVENT_WALK_ASSIST_ACTIVE); or led_sequence_play_next_until(LED_EVENT_WALK_ASSIST_ACTIVE); when the state starts

then when the state ends

led_sequence_cancel_play_until();

[rananna]

yes of course! you added this function some time ago for me. I will do this. However, I think there may still be a bug if you fill up the led buffer and call repeatedly led_play sequence() I think you should not get any problems beyond 15 calls, but I am getting some strange (probably memory related) issues. Could you please try that with a test loop and see how it behaves for you?

On Wed, Feb 24, 2021 at 3:05 PM 4var1 notifications@github.com wrote:

led_sequence_play_now_until(LED_EVENT_WALK_ASSIST_ACTIVE); or led_sequence_play_next_until(LED_EVENT_WALK_ASSIST_ACTIVE); when the state starts

then when the state ends

led_sequence_cancel_play_until();

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenSourceEBike/TSDZ2_wireless/pull/100#issuecomment-785338986, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGZW2IUBIQ4KOMKQBCTCDELTAVLZVANCNFSM4YCQMHHQ .

[4var1]

yes of course! you added this function some time ago for me. I will do this. However, I think there may still be a bug if you fill up the led buffer and call repeatedly led_play sequence() I think you should not get any problems beyond 15 calls, but I am getting some strange (probably memory related) issues. Could you please try that with a test loop and see how it behaves for you? … On Wed, Feb 24, 2021 at 3:05 PM 4var1 @.***> wrote: led_sequence_play_now_until(LED_EVENT_WALK_ASSIST_ACTIVE); or led_sequence_play_next_until(LED_EVENT_WALK_ASSIST_ACTIVE); when the state starts then when the state ends led_sequence_cancel_play_until(); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#100 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGZW2IUBIQ4KOMKQBCTCDELTAVLZVANCNFSM4YCQMHHQ .

Ok - i'll take a look - all buffers i've implemented are ringbuffers so if there was a problem it should also occur after a while anyway than when the buffer was full - as the buffer continually wraps but I'll take a look.

[4var1]

yes of course! you added this function some time ago for me. I will do this. However, I think there may still be a bug if you fill up the led buffer and call repeatedly led_play sequence() I think you should not get any problems beyond 15 calls, but I am getting some strange (probably memory related) issues. Could you please try that with a test loop and see how it behaves for you? … On Wed, Feb 24, 2021 at 3:05 PM 4var1 @.***> wrote: led_sequence_play_now_until(LED_EVENT_WALK_ASSIST_ACTIVE); or led_sequence_play_next_until(LED_EVENT_WALK_ASSIST_ACTIVE); when the state starts then when the state ends led_sequence_cancel_play_until(); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#100 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGZW2IUBIQ4KOMKQBCTCDELTAVLZVANCNFSM4YCQMHHQ .

can you tell me what the symptoms are that you think are memory issues?

[rananna]

Well, when I pumped the buffer full many times over, the light continued to flash ok, but when I issued the command to stop, the leds then  refused to work for subsequent commands. On Feb 24, 2021, 5:43 PM -0500, 4var1 , wrote:

yes of course! you added this function some time ago for me. I will do this. However, I think there may still be a bug if you fill up the led buffer and call repeatedly led_play sequence() I think you should not get any problems beyond 15 calls, but I am getting some strange (probably memory related) issues. Could you please try that with a test loop and see how it behaves for you? … On Wed, Feb 24, 2021 at 3:05 PM 4var1 @.***> wrote: led_sequence_play_now_until(LED_EVENT_WALK_ASSIST_ACTIVE); or led_sequence_play_next_until(LED_EVENT_WALK_ASSIST_ACTIVE); when the state starts then when the state ends led_sequence_cancel_play_until(); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#100 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGZW2IUBIQ4KOMKQBCTCDELTAVLZVANCNFSM4YCQMHHQ . can you tell me what the symptoms are that you think are memory issues? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[rananna]

OK, maybe this will help track the issue down.

@4var1,

I put led_sequence_play_next_until(LED_EVENT_SHORT_RED); on a button press (the brake pin) and led_sequence_cancel_play_until(); on the button release (brake released) all works fine, UNLESS you pump the brakes! (ie: generate multiple interrupts quickly) In this scenario, the leds stop working and the board hangs up, requiring a reset to get going again. (i also tried led_sequence_play_until(LED_EVENT_SHORT_RED); on the button press, but get the same result.

This should be easier for you to reproduce; please give it a try .

[rananna]

In fact, all you have to do is press the button once quickly, generating a button press and release very fast. This seems to cause the hang up

[rananna]

OK, forget all the above. The issue was me calling the ANT LEV commands too quickly in succession when the button was pressed quickly. This caused the hangup of the board! Nothing to do with the LEDS Sorry to bother you on this one.