STMicroelectronics / x-cube-subg2

X-CUBE-SUBG2 is an expansion software package for STM32Cube. The software runs on the STM32 and includes drivers that recognize the Sub-1 GHz RF communication for S2-LP.
https://www.st.com/en/embedded-software/x-cube-subg2.html
Other
5 stars 3 forks source link

S2LP Driver: fixed code errors which break compilation #4

Closed samuelsd1 closed 2 years ago

samuelsd1 commented 2 years ago

Replaced all calls to S2LPSpiReadRegisters with S2LP_ReadRegister, and all calls to S2LPSpiWriteRegisters with S2LP_WriteRegister. This fixes the undefined reference to the replaced functions.

Removed duplicate enum definition ModeExtRef, which was originally defined in s2lp.h, while leaving a single definition in s2lp_general.h, to align with the intent of s2lp_general files.

Defined macro for s_assert_param as it was undefined. Later this macro should be removed (see the additional context of #3).

Included s2lp_general.h in s2lp.h. Then removed duplicate functions defined in s2lp_general.h:

Moved the g_xStatus declaration to s2lp.h, and removed the duplicate defitintion of g_xStatus from s2lp_types.c. It is more reasonable to define it in s2lp.h and s2lp.c, since this is a variable and not a type. This aligns with the definition of the definition of S2LP_RefreshStatus in the s2lp.c file, as well as IO_funcs variable in s2lp.c.

Removed the duplicate S2LPGeneralLibraryVersion() macro from s2lp_general.h.

This patch was successfully compiled in STM32CubeIde 1.8.0, and was tested on a custom board with simple P2P app.

Fixes: https://github.com/STMicroelectronics/x-cube-subg2/issues/3

IMPORTANT INFORMATION

Contributor License Agreement (CLA)

mgrella commented 2 years ago

Thank you for your feedback.

samuelsd1 commented 2 years ago

@mgrella Thanks for your reply :)

Yes, from a functional point of view, excluding s2lp_general.h, s2lp_general.c and s2lp_types.c seems to work. If so' the file s2lp_types.c should be removed. In my pull request I assumed that it was intended to stay, so I didn't remove it - but removing it will be simple and correct. About the s2lp_general files - they seem to have sense, so I refactored the code to include them.

Should I add a updated commit which will remove s2lp_types.c and leave s2lp_general files? Or you would prefer instead to completely remove s2lp_general files from this repo?

Best regards, David

mgrella commented 2 years ago

Hello @samuelsd1 ,

as I explained in #3 we are going to publish an update of the x-cube-subg2 that will also contain a different wrapping of original S2-LP Driver (of course it will contain the fix you proposed with #2). So most of the context of this pull request will be automatically solved, we could then check if and what still needs to be fixed.

BR, Marco

mgrella commented 2 years ago

Closing this PR, S2-LP Component Driver now contains official S2-LP Library from S2-LP DK. We'll check the other feedbacks for further updates in following months