@fpistm glad I could help! Any estimate on when is the new version going to be released?

@fpistm glad I could help! Any estimate on when is the new version going to be released?

Hi@jgromes
I will do a 2.7.1 release for bugfix.
Anyway, this PR and#2193 is not correct.
Proper way to define is:
SPISettings(16000000, MSBFIRST, SPI_MODE0)

I will do a proper fixes on core side, anyway the newSPISettings aligned with the ArduinoCore-API does not work with RadioLib.
It seems the values is not evaluated when theSTM32WLx radio = new STM32WLx_Module(); is done.
SubGhz.spi_settings is not evaluate yet:
https://github.com/jgromes/RadioLib/blob/81c59f61ff69749be9579a3566dc55d3bfb765b8/src/modules/SX126x/STM32WLx_Module.cpp#L27C25-L27C25

Replacing bySPISettings(16000000, MSBFIRST, SPI_MODE0) solve the problem.
I have to dig into this to propose a proper fix.
Maybe I will have to do a PR to RadioLib. If you have any idea do not hesitate. 😜

@fpistm

It seems the values is not evaluated when the STM32WLx radio = new STM32WLx_Module(); is done.

Not sure what you mean - I can compile RadioLib STM32WL example fine with the changes from both PRs.

@fpistm

It seems the values is not evaluated when the STM32WLx radio = new STM32WLx_Module(); is done.

Not sure what you mean - I can compile RadioLib STM32WL example fine with the changes from both PRs.

It compiles but does not work.

Ah, I see. Odd, I can't test that unfortunately, since I don't have any STM32WL on hand. Maybe@matthijskooijman would be able to help?

I've read up a bit and am trying to make sense of the changes. I think that:

• 392469a Syncs the SPISettings API with ArduinoCore-API, adding some methods and changing the initialization. This makes the constructor no longerconstexpr, and removes theSPI_MODE_x constants (which actually were an implementation detail, the actual API should have usedSPI_MODEx, but since the values were the same using the wrong enum did actually work).
• fix(SubGhz): SPISettings not properly defined #2193 changes the SubGhz SPISettings object to not beconstexpr, which is no longer possible now SPISettings constructor is notconstexpr anymore.
• This PR changes the use ofSPI_MODE_x toSPI_MODEx so it uses the correct API.

Then, what@fpistmis saying is that by the time radiolib usesSubGhz.spi_settings, it is now actually initialized yet. This is because both are part of global object initialization, and there is no guarantee about relative ordering of global objects in different compilation units. This was not previously a problem, sinceconstexpr objects are evaluated at compiletime andare initialized before everything else (by copying bytes from the.data segment instead of running code).

Wrt to392469a, I wonder if all these changes are really needed. Personally, I would like to keep the constructor constexpr, which ensures that constructing an SPISettings object can be done at compiletime. The entire alwaysInline/mightInline stuff really stems from the AVR implementation, where this was, IIRC, needed because there constructing an SPISettings object would condense the settings into a single uint8_t value ready to be put into the hardware registers directly, to ensure that the SPISettings approach was just as compact as the old (non-portable) method of specifying the register values directly. I think that approach also predates theconstexpr concept, so maybe the inline stuff isn't actually really needed in AVR anymore either.

In any case - I think the inline stuff has no place in ArduinoCore-API, since it really does not add anything (in fact, I wonder about the definition of this class in the API repo at all, since any core that includes ArduinoCore-API now can no longer define an optimized SPISettings class like AVR does now - I think ArduinoCore-API should just define an expected API and let cores implement that however they want).

So I would suggest stripping the alwaysInline/mightInline stuff from SPISettings - it adds nothing, doing the intialization directly in the constructor and making itconstexpr again. Then reverting#2193 and keeping this PR should, I think make things work as before.

I've got to run now, but if it is helpful I could have a closer look and/or test somewhere next week?

Thanks@matthijskooijman.
Agreed, that's the way I thought to fix the issue and avoid other regressions.

@jgromes could you file an issue to revert the SPISettings as constexpr, please ?