Intercept the usart isr without resorting to hacking the core

Post here first, or if you can't find a relevant section!
Riva
Posts: 40
Joined: Fri May 06, 2016 6:42 am

Intercept the usart isr without resorting to hacking the core

Post by Riva » Thu Feb 16, 2017 5:16 pm

I'm working on code (that may eventually become a library) to implement the DMX512 lighting protocol on a Maple Mini.

Barring a few teething problems the proof of concept TX code is written and working so I'm now concentrating on the RX code and have hit a snag where you cannot intercept/replace the usart interrupt vectors defined in usart_f1.c without editing the file and removing the definition(s) for the usart you want to control.
The Maple docs said somewhere that they should be made __weak so they can be replaced without having to hack the core files but this never happened and I have no real idea how to go about this.
Just preceding the usart_f1.c definition with __weak works fine as long as I write interrupt code to deal with it but if you don't have your own interrupt code I would have expected/hoped the original definition would prevail but it does not and the MCU locks up.

Code: Select all

/*
 * Interrupt handlers.
 */

 void __irq_usart1(void) {
    usart_irq(&usart1_rb, &usart1_wb, USART1_BASE);
}

__weak void __irq_usart2(void) {
    usart_irq(&usart2_rb, &usart2_wb, USART2_BASE);
}
Is there something I am missing (C++ is not my strongest language) in using the __weak attribute or maybe a better way to do it?

victor_pv
Posts: 1483
Joined: Mon Apr 27, 2015 12:12 pm

Re: Intercept the usart isr without resorting to hacking the core

Post by victor_pv » Thu Feb 16, 2017 6:11 pm

Riva wrote:I'm working on code (that may eventually become a library) to implement the DMX512 lighting protocol on a Maple Mini.

Barring a few teething problems the proof of concept TX code is written and working so I'm now concentrating on the RX code and have hit a snag where you cannot intercept/replace the usart interrupt vectors defined in usart_f1.c without editing the file and removing the definition(s) for the usart you want to control.
The Maple docs said somewhere that they should be made __weak so they can be replaced without having to hack the core files but this never happened and I have no real idea how to go about this.
Just preceding the usart_f1.c definition with __weak works fine as long as I write interrupt code to deal with it but if you don't have your own interrupt code I would have expected/hoped the original definition would prevail but it does not and the MCU locks up.

Code: Select all

/*
 * Interrupt handlers.
 */

 void __irq_usart1(void) {
    usart_irq(&usart1_rb, &usart1_wb, USART1_BASE);
}

__weak void __irq_usart2(void) {
    usart_irq(&usart2_rb, &usart2_wb, USART2_BASE);
}
Is there something I am missing (C++ is not my strongest language) in using the __weak attribute or maybe a better way to do it?
Do you need to completely replace the USART irq handler, or you just need to add to the handler?
in the second case, perhaps you could modify the core to allow it to attach an additional function to be called before or after the core handler, similar to the DMA interrupts, where there is a handler that will do a few things, and then called a user defined handler if it has been attached.

For the first case, I would have thought __weak should work, but since the symbols (__irq_usart1, etc) are already declared weak in the interrupt table, perhaps that makes the __weak not work fine in the function?
See https://github.com/rogerclarkmelbourne/ ... nce/isrs.S

There was another way to declare those symbols as weak, which I applied last year for something (don't fully remember what was it for, but i think was related to PIN_MAP in ram, which we resolved a different way), you can see in the history of that file how it was using weakref.

You can test to replace that file with the version that used weakrefs, and see if in that case the __weak parameter in the function works as expected.

EDIT:
I was checking when and why we tested weakrefs. I din't read the whole story, but it had to do with some errors when -Wall was removed from the linker parameters.
In one of the posts, Rick linked to this file:
https://github.com/leaflabs/libmaple/bl ... rrupts.txt
Which contains this paragraph:
IMPORTANT: the function you define MUST HAVE C LINKAGE. C++ name
mangling will confuse the linker, and it won't find your function. So
if you're writing your IRQ handler in a C++ file, you need to define
it like this:

extern "C" void __irq_sdio(void) {
// etc.
}

Riva
Posts: 40
Joined: Fri May 06, 2016 6:42 am

Re: Intercept the usart isr without resorting to hacking the core

Post by Riva » Fri Feb 17, 2017 9:29 am

victor_pv wrote: Do you need to completely replace the USART irq handler, or you just need to add to the handler?
in the second case, perhaps you could modify the core to allow it to attach an additional function to be called before or after the core handler, similar to the DMA interrupts, where there is a handler that will do a few things, and then called a user defined handler if it has been attached.
Ideally I want to replace the default handler but using a hook could be an option as long as the default IRQ code can be told to process/ignore depending on a hook return code.
For the first case, I would have thought __weak should work, but since the symbols (__irq_usart1, etc) are already declared weak in the interrupt table, perhaps that makes the __weak not work fine in the function?
__weak works fine as long as I define IRQ handlers but if I don't define one I would expect the default to be used instead but it seems the default one is not linked so any code that uses Serial just locks up.

There was another way to declare those symbols as weak, which I applied last year for something (don't fully remember what was it for, but i think was related to PIN_MAP in ram, which we resolved a different way), you can see in the history of that file how it was using weakref.
This is all getting a bit to advanced for my C++ skills as despite reading here I'm not the wiser what the difference is between weak & weakref.

You can test to replace that file with the version that used weakrefs, and see if in that case the __weak parameter in the function works as expected.
Will give it a try and report back.

EDIT:
I was checking when and why we tested weakrefs. I din't read the whole story, but it had to do with some errors when -Wall was removed from the linker parameters.
In one of the posts, Rick linked to this file:
https://github.com/leaflabs/libmaple/bl ... rrupts.txt
Which contains this paragraph:
IMPORTANT: the function you define MUST HAVE C LINKAGE. C++ name
mangling will confuse the linker, and it won't find your function. So
if you're writing your IRQ handler in a C++ file, you need to define
it like this:

extern "C" void __irq_sdio(void) {
// etc.
}
Maybe I'm doing it wrong then as I have a

Code: Select all

extern "C" 
{
   void __irq_usart2(void);
}
in the start of my sketch and then later on the actual function.

Code: Select all

// usart2 interrupt
void __irq_usart2()			
{	
    if(USART2_BASE->SR & USART_SR_LBD)                                // RX line break detected
    {
    USART2_BASE->SR &= ~ USART_SR_LBD;                              // Clear line break error flag 
    }
  usart2SR = USART2_BASE->SR;                                       // Copy status register before reading the data register else flags may be cleared
  if ((USART2_BASE->CR1 & USART_CR1_RXNEIE) && (usart2SR & USART_SR_RXNE)) // Expecting USART RX Interrupt AND have a byte to read?

{SNIP SNIP SNIP}
}                

Riva
Posts: 40
Joined: Fri May 06, 2016 6:42 am

Re: Intercept the usart isr without resorting to hacking the core

Post by Riva » Fri Feb 17, 2017 11:07 am

@victor_pv Resorting to a phrase way to young for me "You're the man".
You can test to replace that file with the version that used weakrefs, and see if in that case the __weak parameter in the function works as expected.
Will give it a try and report back.
Changing the .set to .weakref for the usart handlers in the isrs.S file and adding __weak to the interrupt handlers in usart_f1.c mean I seem to be able to intercept the default IRQ within code and if I don't intercept then the original code still works.
Is this something worth considering for a core change? Subject to more thorough testing.

victor_pv
Posts: 1483
Joined: Mon Apr 27, 2015 12:12 pm

Re: Intercept the usart isr without resorting to hacking the core

Post by victor_pv » Fri Feb 17, 2017 12:27 pm

Riva wrote:@victor_pv Resorting to a phrase way to young for me "You're the man".
You can test to replace that file with the version that used weakrefs, and see if in that case the __weak parameter in the function works as expected.
Will give it a try and report back.
Changing the .set to .weakref for the usart handlers in the isrs.S file and adding __weak to the interrupt handlers in usart_f1.c mean I seem to be able to intercept the default IRQ within code and if I don't intercept then the original code still works.
Is this something worth considering for a core change? Subject to more thorough testing.
We probably will need to find why we discarded it in the first place. I think it was because it didn't solve the problem we were trying to solve, but may have been because it caused others. Try to see if it causes you any more problems, when I have time I will see if I can read the whole conversation where we did it.
It was originally Rick's idea to solve something, then I applied it to more things, then something went wrong and we discarded that change.
If we still use -Wall as one of the linker scripts, then we discarded it just because it didn't solve the need to use that and we still needed to use it, so no point making a change that didn't add a real benefit at the time.

victor_pv
Posts: 1483
Joined: Mon Apr 27, 2015 12:12 pm

Re: Intercept the usart isr without resorting to hacking the core

Post by victor_pv » Fri Feb 17, 2017 5:11 pm

Riva wrote:@victor_pv Resorting to a phrase way to young for me "You're the man".
You can test to replace that file with the version that used weakrefs, and see if in that case the __weak parameter in the function works as expected.
Will give it a try and report back.
Changing the .set to .weakref for the usart handlers in the isrs.S file and adding __weak to the interrupt handlers in usart_f1.c mean I seem to be able to intercept the default IRQ within code and if I don't intercept then the original code still works.
Is this something worth considering for a core change? Subject to more thorough testing.
Are you using this?

Code: Select all

	.weakref __exc_nmi, __default_handler
	.globl	__exc_nmi
or this?

Code: Select all

.weak	__exc_nmi
.globl	__exc_nmi
.weakref __exc_nmi, __default_handler

User avatar
RogerClark
Posts: 6413
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Intercept the usart isr without resorting to hacking the core

Post by RogerClark » Sat Feb 18, 2017 1:13 am

If we can work out whether there are any downsides to making these into weak references, I"d be happy to accept a PR to change this.

But we'd need to make very sure it doesnt break something ;-)

victor_pv
Posts: 1483
Joined: Mon Apr 27, 2015 12:12 pm

Re: Intercept the usart isr without resorting to hacking the core

Post by victor_pv » Sat Feb 18, 2017 4:03 am

RogerClark wrote:If we can work out whether there are any downsides to making these into weak references, I"d be happy to accept a PR to change this.

But we'd need to make very sure it doesnt break something ;-)
We didn't :( I think this would need extensive testing, but I dont see a reason to hurry it if it's only needed for this particular problem.

User avatar
RogerClark
Posts: 6413
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: Intercept the usart isr without resorting to hacking the core

Post by RogerClark » Sat Feb 18, 2017 6:15 am

Hi Victor

I'm not suggesting extensive testing, but we have been caught in the past by changing things in the past, which had unintended consequences.

racemaniac
Posts: 424
Joined: Sat Nov 07, 2015 9:09 am

Re: Intercept the usart isr without resorting to hacking the core

Post by racemaniac » Sat Feb 18, 2017 6:34 am

victor_pv wrote:
RogerClark wrote:If we can work out whether there are any downsides to making these into weak references, I"d be happy to accept a PR to change this.

But we'd need to make very sure it doesnt break something ;-)
We didn't :( I think this would need extensive testing, but I dont see a reason to hurry it if it's only needed for this particular problem.
trust me, you're not the only one changing the core to get to interrupts XD
i'd also love this change :D

Post Reply

Who is online

Users browsing this forum: forgatten2 and 1 guest