Pass PIOASM_EXTRA_SOURCE_FILES to Pioasm sub-cmake and add OUTPUT_FORMAT option (fixes #827) #828

2022-05-14T15:38:25-05:00

Ferdi265 commented

2022-05-14 15:38:25 -05:00

(Migrated from github.com)

This PR changes FindPioasm.cmake to pass through the PIOASM_EXTRA_SOURCE_FILES variable, so that it is actually able to be changed by users of the SDK without modifying it.

This PR also adds an OUTPUT_FORMAT option to pico_generate_pio_header that sets the output format used so users can also easily use their custom output formats.

Example usage:

file(GLOB pioasm-sources pioasm-extensions/*.cpp)
set(PIOASM_EXTRA_SOURCE_FILES ${pioasm-sources})

pico_generate_pio_header(my-target thing.pio OUTPUT_FORMAT sdk-custom)

This PR changes `FindPioasm.cmake` to pass through the PIOASM_EXTRA_SOURCE_FILES variable, so that it is actually able to be changed by users of the SDK without modifying it. This PR also adds an OUTPUT_FORMAT option to `pico_generate_pio_header` that sets the output format used so users can also easily use their custom output formats. Example usage: ``` file(GLOB pioasm-sources pioasm-extensions/*.cpp) set(PIOASM_EXTRA_SOURCE_FILES ${pioasm-sources}) pico_generate_pio_header(my-target thing.pio OUTPUT_FORMAT sdk-custom) ```

kilograham (Migrated from github.com) reviewed

2022-05-15 10:49:01 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

				endfunction()

The variable name should be something like PIOASM_DEFAULT_OUTPUT_FORMAT

The variable name should be something like `PIOASM_DEFAULT_OUTPUT_FORMAT`

Ferdi265 (Migrated from github.com) reviewed

2022-05-15 11:32:47 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

				endfunction()

I just noticed a typo in there: it says "usdk_generate_pio_header_OUTPUT_FORMAT", which is incorrect, should be "pico_...". That variable is set by the cmake_parse function above and is just a temporary.

IMO it could make sense to set the output format to PIOASM_DEFAULT_OUTPUT_FORMAT if the OUTPUT_FORMAT option isn't set (as opposed to hardcoding c-sdk) as an additional way to customize it.

I'll fixup the patch series to fix the typo, and I can also add the PIOASM_DEFAULT_OUTPUT_FORMAT variable if you want.

I just noticed a typo in there: it says "usdk_generate_pio_header_OUTPUT_FORMAT", which is incorrect, should be "pico_...". That variable is set by the cmake_parse function above and is just a temporary. IMO it could make sense to set the output format to PIOASM_DEFAULT_OUTPUT_FORMAT if the OUTPUT_FORMAT option isn't set (as opposed to hardcoding c-sdk) as an additional way to customize it. I'll fixup the patch series to fix the typo, and I can also add the PIOASM_DEFAULT_OUTPUT_FORMAT variable if you want.

Ferdi265 (Migrated from github.com) reviewed

2022-05-15 11:40:03 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

				endfunction()

typo fixed in a8ce4cd
default variable added in 546b89d

typo fixed in a8ce4cd default variable added in 546b89d

kilograham (Migrated from github.com) reviewed

2022-05-15 15:00:57 -05:00

kilograham (Migrated from github.com) left a comment

Can you please rename PIOASM_DEFAULT_OUTPUT_FORMAT to PICO_DEFAULT_PIOASM_OUTPUT_FORMAT and also add a line

# PICO_DEFAULT_PIOASM_OUTPUT_FORMAT, default output format used by pioasm when using pico_generate_pio_header, group=build

above the pico_generate_pio_header (that way it will be automatically added to tthe docs!)

Can you please rename `PIOASM_DEFAULT_OUTPUT_FORMAT` to `PICO_DEFAULT_PIOASM_OUTPUT_FORMAT` and also add a line ``` # PICO_DEFAULT_PIOASM_OUTPUT_FORMAT, default output format used by pioasm when using pico_generate_pio_header, group=build ``` above the pico_generate_pio_header (that way it will be automatically added to tthe docs!)

Ferdi265 commented

2022-05-15 15:10:13 -05:00

(Migrated from github.com)

sure!

lurch (Migrated from github.com) reviewed

2022-05-16 11:49:25 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

Could you please add this to the end of the line too?

, default=c-sdk

Could you please add this to the end of the line too? ``` , default=c-sdk ```

kilograham (Migrated from github.com) reviewed

2022-05-16 13:46:14 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

i'm going to merge it now; @lurch you can add a PR to update that!

kilograham (Migrated from github.com) approved these changes

2022-05-16 14:20:28 -05:00

tools/CMakeLists.txt

					
				@ -4,9 +4,18 @@ function(_pico_init_pioasm)

				    find_package(Pioasm REQUIRED)

oh, found handy new edit feature in github!

Sign in to join this conversation.

No reviewers

kilograham

No labels

compiler version issues

needs reporter detail

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-raspberrypi/pico-sdk!828

				`@ -4,9 +4,18 @@ function(_pico_init_pioasm)`
				`find_package(Pioasm REQUIRED)`

Rows
Columns