Purpose of 'modm_weak' on AVR ADC ISR?

[ghost]

Hello,

I am fairly new to modm, having happened upon it only a week or two ago. I am impressed with the scope and scale of the project, and am trying to improve my proficiency with C++ by studying it.

Being more familiar with the AVR than most of the other targets, I have built a number of the examples that had some similarities to projects I did in the past. I like to create a listing so I can pick apart what is going on.

One of these examples, 'examples/avr/adc/oversample', reads a number of ADC channels repeatedly and averages the results. A pointer to the interrupt handler (sampleAdc()) is stored in 'handler', which is accessed by the ISR when the interrupt occurs; or that's what is supposed to happen.

When I looked at the listing, I found 'sampleAdc()' and 'handler', and the pointer was stored as expected. But there was no vector created in the vector table for the ADC, and I found that nothing actually called 'sampleAdc'.

Looking in 'platform/adc /at90_tiny_mega/adc_interrupt.cpp.in', I found the declaration for the ISR to be 'MODM_ISR(ADC, modm_weak)'. If I removed the 'modm_weak' and recompiled, the vector was created, and the ISR followed the pointer to 'sampleAdc()' as expected.

So, why have an ISR with a weak attribute? In this case, at least, it seems to cause a problem; but I assume there was a reason for doing it that way.

Thanks, Tom Rush

[salkinium]

That's just a bug 😬.

I guess the intention was to let the application overwrite the library definitions, but that does not work since the library is already overwriting the GCC-provided linkerscript, which defines the interrupts as weakly linked already.
So somewhere there needs to be a strongly linked definitions or it doesn't get linked.

Do you want to open a PR with the modm_weak removed?

[ghost]

That's just a bug

I kinda figured that, but I'm not really up on how weak symbols are used, so thanks for the explanation.

Do you want to open a PR with the modm_weak removed?

Sure, but since I'm a newbie here--do I have to do a pull request and all that?

[salkinium]

No, it was more like an offer so that you can see how a PR works and then you have more agency for future fixes ;-P
You can even edit the file online and make a PR that way: https://github.com/modm-io/modm/edit/develop/src/modm/platform/adc/at90_tiny_mega/adc_interrupt.cpp.in

[salkinium]

Also you get properly credited for your bug report and fix ;-P

[ghost]

Okay, I did the online edit option. I'm not expecting a lot of 'future fixes', but who knows :-)