Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Examples for Building and Using Static Libraries #476

Merged
merged 11 commits into from
Apr 7, 2023

Conversation

ttmut
Copy link
Contributor

@ttmut ttmut commented Mar 9, 2023

This PR adds an example project that shows how to build part of the code as a static library.

@ttmut ttmut requested a review from ozersa March 9, 2023 09:48
@ozersa ozersa requested a review from Jake-Carter March 10, 2023 12:16
Copy link
Contributor

@Jake-Carter Jake-Carter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great example, but I think the implementation is too complicated.

You can leverage the make lib rule and achieve the same result with a recursive make call.

Below is an example that builds the project in LIB_LOCATION as a static lib and adds it to the project's build. It also builds everything in the project's local folder to address the make clean issues.

# This file can be used to set build configuration
# variables.  These variables are defined in a file called
# "Makefile" that is located next to this one.

# For instructions on how to use this system, see
# https://analog-devices-msdk.github.io/msdk/USERGUIDE/#build-system

# **********************************************************

# Move all build products to project's build folder
BUILD_DIR = $(CURDIR)/build
LIB_BUILD_DIR = $(BUILD_DIR)/build_lib

# Point to the location of the library
LIB_LOCATION = ../GPIO

# Add library to the build
LIB_NAME = mylib
MYLIB = $(LIB_BUILD_DIR)/$(LIB_NAME).a
LIBS += $(MYLIB)

# Add rule to build library with a recursive make call
$(MYLIB):
    make -C $(LIB_LOCATION) BUILD_DIR=$(LIB_BUILD_DIR) PROJECT=$(LIB_NAME) lib

@ttmut ttmut requested review from Jake-Carter and ozersa March 16, 2023 12:33
Copy link
Contributor

@ozersa ozersa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally read me file content should be clear as possible as.
It should mention testlib project just genarate .a file...

@ozersa
Copy link
Contributor

ozersa commented Mar 21, 2023

Additionally due to there are two examples, renaming them as below will be better.

  • Library_Generate
  • Library_Use

printf("SW2 then toggles the red and green LEDs according to the read values.\n");

while (1) {
for (i = 0; i < num_pbs && i < num_leds; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, simple is better, just blinkig LED0 is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@ttmut ttmut requested a review from ozersa March 22, 2023 11:00
printf("switch the red LED on as long as the push button is pressed.\n");

while (1) {
if (gpio_get(&pb_pin[0])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, it is better now lets make it more simple.
Because current flow expect user interaction (pressing button) to visualize something (blink led).
Our some boards have multiple buttons that too near each other (especially fthr boards). RST, SW1, SW2...
In that case it will confuse user.
So that demonstrating somethings without any user interaction will be better instead of waiting action by user.

Just blinking led would be better, exactly as Hello_World example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

SBT=0

# Point to the location of the library
LIB_LOCATION = ../Library_Generate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add .a and library interface file (.h file) to this project as statically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does static inclusion imply in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

LIB_NAME = Library_Generate
LIB_BUILD_DIR = $(LIB_LOCATION)/build
MYLIB = $(LIB_BUILD_DIR)/$(LIB_NAME).a
LIBS += $(MYLIB)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of .a file should be myLib.a or testLib.a.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@ttmut ttmut requested a review from ozersa March 22, 2023 14:19
@ozersa
Copy link
Contributor

ozersa commented Mar 22, 2023

Thanks approved.

@Jake-Carter could you provide your feedback too,

Then this development can be propagated to other MCUs.

@@ -0,0 +1,64 @@
/**
* @file gpiolib.h
Copy link
Contributor

@Jake-Carter Jake-Carter Mar 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gpiolib.h should not be copied into Library_Use.

Add the Library_Generate project to IPATH in the Library_Use project.mk file. The safest way is to reference MAXIM_PATH. Ex:

IPATH += $(MAXIM_PATH)/Examples/$(TARGET_UC)/Library_Generate

Libraries normally provide an Include folder; you wouldn't usually make a copy of the include files to use the library.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment but these examples are independent examples.

The purpose of Library_Generate is to demonstrate the step how user create a library.
The purpose of Library_Use is to demonstrate the steps how user use a library that generated by another project.

Making a link to "C:/MaximSDK/Examples/xxx/Library_Generate" not a good idea on my view.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ozersa
Ok, even in this case I still think that having myLib.a inside the Library_Use might not be the best practice. Consider the case where a library is external to the project. The user must add:

IPATH += #library include path
PROJ_LDFLAGS += -L#path to library file
LIBS += #library file

However the example only demonstrates LIBS += #library file. It's rare that the user will copy the entire library static file and include files directly into the root of their project

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, to demonstrate usage of IPATH, PROJ_LDFLAGS too, lets put lib related files under "lib" folder instead of root folder.

@ttmut please create below folders in Library_Use example

  • lib
  • lib/include
    folders and put .a and .h in these folder, apply related changes on the project.mk files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could only make this work with PROJ_LIBS and PROJ_LDFLAGS as shown below. If I am to use LIBS, PROJ_LDFLAGS becomes redundant.

IPATH += lib/include
PROJ_LDFLAGS += -Llib
PROJ_LIBS += myLib
IPATH += lib/include
PROJ_LDFLAGS += -Llib #Unnecessary since lib directory is already included in LIBS
LIBS += lib/libmyLib.a

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'm not sure why the core makefiles have both LIBS and PROJ_LIBS. I think the original intention was LIBS would be internal to the SDK. We could streamline this at some point in the future.

PROJ_LIBS is the best choice for this. Looks good!

Examples/MAX32650/Library_Use/myLib.a Outdated Show resolved Hide resolved
Examples/MAX32650/Library_Use/project.mk Outdated Show resolved Hide resolved
Examples/MAX32650/Library_Use/README.md Show resolved Hide resolved
Examples/MAX32650/Library_Generate/README.md Show resolved Hide resolved
@ozersa
Copy link
Contributor

ozersa commented Mar 31, 2023

It is ok for me, can be propagated to other MCUs.

ttmut added 4 commits April 3, 2023 12:47
Added an example project that builds and links a static library.
Rename Makefile to libExample.make so that the test build script does
not attempt to run make on it.
 * Rename StaticLibrary to TestLib
 * Reorganize the project makefiles
 * Replace the library api with GPIO functions
ttmut added 7 commits April 3, 2023 12:47
Rename TestLib to Library_Generate and StaticLibrary_Example to
Library_Use.

 * Set Library_Generate default target as lib
 * Eliminate relative include paths
 * Add missing newlines
 * Simplify example application
 * Update documentation
Including the previously generated library to prevent creating a
dependency to another project.

In the main application, toggle LED every 500ms, do not check the
state of the push buttons.
Move myLib.a to lib/ and tell the linker the location of the library
using PROJ_LDFLAGS. Rename myLib.a to libmyLib.a so that it can be
passed to linker with the -l option.

Remove documentation for SBT option in Library_Generate project since it
does not output an executable that needs to be signed.
Added Library_Generate and Library_Use examples.
@ozersa
Copy link
Contributor

ozersa commented Apr 3, 2023

After this PR to be merged please add same example on MAX32570 too.

Copy link
Contributor

@Jake-Carter Jake-Carter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ttmut, looks good! Merging this in

@Jake-Carter Jake-Carter changed the title Add static library example Add Examples for Building and Using Static Libraries Apr 7, 2023
@Jake-Carter Jake-Carter merged commit 233c7dd into main Apr 7, 2023
@Jake-Carter Jake-Carter deleted the dev/static_lib branch April 7, 2023 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants