Print device by family function #205

robamu · 2021-06-08T17:18:00Z

I added a function which simply uses the existing stm32_get_devices_by_family to print out
the devices without returning them. I also updated the documentation accordingly.

atsju

I just tried the code out and
stm32_print_devices_by_family(FAMILY STM32F4) returns nothing. I figured out that call must be stm32_print_devices_by_family(FAMILY F4). There should be a warning about unknown family in stm32_get_devices_by_family and maybe just some little more documentation.

Also I do not see why the function needs FAMILY as first keyword as it's in the name of the function. So maybe we could simplify the call.
If I omit FAMILY just all device get printed instead of an error that I didn't call function properly.

I think this needs a bit of rework before being merged. Maybe even stm32_get_devices_by_family must be reworked.

Maybe I'm a bit too extreme but we could completely remove file devices.cmake. This is typically something not scalable and not maintainable. And I do not see the practical use for it as we (are supposed to) support almost all STM32 devices and this list may even not fully know what we support. @Hish15 @rmspacefish what do you think of removing the file ?

edit: @rmspacefish I see you use the function in #206 but I'm still worried because of how device list is maintained.

robamu · 2021-06-26T18:13:34Z

I will try to add the warning if the specified family is unknown. I actually found this function useful to find the right target name for my specific board quickly, because the documentation does not have a similar list. I added some more documentation in the README, so I guess you mean documentation in the code? I can add that as well. I actually like the feature that it simply prints all families with no families specified.

robamu · 2021-06-28T08:20:43Z

Warning is now emitted if the device family is invalid. Tested in example
Added documentation in CMakeLists

…family

atsju · 2021-07-14T16:01:24Z

README.md

+* `stm32_get_devices_by_family(DEVICES [FAMILY <family>])` - return into `DEVICES` all supported devices by family (or all devices if `<family>` is empty)
+* `stm32_print_devices_by_family([FAMILY <family>])` - Print all supported devices by family (or all devices if `<family>` is empty)


For me this makes more sense.

Suggested change

* `stm32_get_devices_by_family(DEVICES [FAMILY <family>])` - return into `DEVICES` all supported devices by family (or all devices if `<family>` is empty)

* `stm32_print_devices_by_family([FAMILY <family>])` - Print all supported devices by family (or all devices if `<family>` is empty)

* `stm32_get_devices_by_family(DEVICES <family>)` - return into `DEVICES` all supported devices by family (or all devices if `<family>` is empty)

* `stm32_print_devices_by_family(<family>)` - Print all supported devices by family (or all devices if `<family>` is empty)

FAMILY is only needed when you have multiple possible keywords.

Hmm I thought FAMILY has to be specified as a keyword. Does it work without it?

No I mean you could change the implementation so that FAMILY is not used.
It brings nothing here.

I think it is okay that way. I think this was done because https://cmake.org/cmake/help/latest/command/cmake_parse_arguments.html is used. Using it also allows easier extensibility (e.g. you still want to parse multiple families, but you also want to pass an option or something like a filter)

Using it also allows easier extensibility (e.g. you still want to parse multiple families, but you also want to pass an option or something like a filter)

OK I understand the point for evolution even if I don't think this function will evolve any soon.
cmake_parse_arguments is fine. I didn't know it before but it permits to do clean things.

I think if you want to have a clean function then all misuse errors should be managed

stm32_print_devices_by_family(H7) # this shall throw error because of no FAMILY given stm32_print_devices_by_family(FAMILY H7 L4) #this should throw error OR change FAMILY to a "multi_value_keywords" stm32_print_devices_by_family(FAMILYH7) # this should throw error unknown keyword or something stm32_print_devices_by_family(FAMILY) # this should throw error missing argument

Hish15

It does make sense to define those functions.
I find the name misleading. In fact the functions are actually stm32_print_devices_from_family or stm32_print_family_devices.
This is minor and will not block the merging of the PR, but it is my 2 cents.

cmake/stm32/devices.cmake

Hish15 · 2021-07-16T07:46:01Z

cmake/stm32/devices.cmake

@@ -1125,9 +1128,15 @@ function(stm32_get_devices_by_family DEVICES)
    if(ARG_FAMILY)
        list(FILTER LIST INCLUDE REGEX "^${ARG_FAMILY}")
    endif()
+    if(NOT LIST)


This bit is weird, where does STM32_ALL_DEVICES come from? this test checks this var, not the LIST var.
I also strongly advice against using LIST as a var name. It's not self explanatory, and it is also a cmake key word!

This bit is weird, where does STM32_ALL_DEVICES come from ?

This is one of the main problems for me. It comes from here : https://github.com/ObKo/stm32-cmake/blob/master/cmake/stm32/devices.cmake
and is used only for these functions. See my very first comment in the PR where I already mentioned it and @rmspacefish seems to have an use the function.

Co-authored-by: Hish15 <pharamhector@gmail.com>

robamu · 2021-07-16T16:30:04Z

I will introduce the safety checks mentioned by @atsju when I find some time. I'll also change the name to something better than LIST

1. More checks 2. Multiple families can now be specified

robamu · 2021-07-20T12:51:20Z

I found out that the function to get the device list is used in FindCMSIS.cmake and in the updated FindFreeRTOS.cmake containing the new CMSIS support. Following updates:

Introduced the sanity checks requested by @atsju and also specified FAMILY as a multi_value_keyword now.
CORE is a new single value keyword in the stm32_get_devices_by_family function now because this keyword is used in FindCMSIS.cmake (though it appears not to be used) which will lead to an empty device list because the parser then assumes CORE and ${CORE} are family keywords.
Renamed the list which is set by the function from DEVICES to STM_DEVICES

atsju · 2021-07-20T13:00:12Z

CORE is a new single value keyword in the stm32_get_devices_by_family function now because this keyword is used in FindCMSIS.cmake (though it appears not to be used) which will lead to an empty device list because the parser then assumes CORE and ${CORE} are family keywords.

Just to mention I have seen this during #235 and I removed the CORE keyword as it is not used. You could remove it now I suppose.

I will have a look at the rest later. Thank you for the update

…family

robamu · 2021-07-20T14:31:02Z

Okay, CORE was removed

devices.cmake is now included like common.cmake as well, so both functions can be used without an extra include.

atsju

I need to dig further and I'm not 100% sure of how this needs to be done. If you let me some time I will try things out and propose some changes. I don't want to abuse of your time by pointing in wrong direction and needing reverse.

atsju · 2021-07-23T15:11:32Z

cmake/stm32/devices.cmake

+        if(NOT ${ARGV0} STREQUAL "STM_DEVICES")
+            message(WARNING "Passed list ${ARGV0} invalid. Still setting STM_DEVICES variable")
+        endif()


this is not what the function is doing. See https://markdewing.github.io/blog/posts/notes-on-cmake/

stm32_get_devices_by_family(retVal FAMILY L4) message(STATUS "retVal is ${retVal}") message(STATUS "STM_DEVICES is ${STM_DEVICES}")

outputs

[build] CMake Warning at C:/GIT/stm32-cmake/cmake/stm32/devices.cmake:1131 (message): [build] Passed list retVal invalid. Still setting STM_DEVICES variable [build] Call Stack (most recent call first): [build] CMakeLists.txt:8 (stm32_get_devices_by_family) [build] [build] [build] -- retVal is L412C8;L412CB;L412K8;L412KB;L412R8;L412RB;L412T8;L412TB;L422CB;L422KB;L422RB;L422TB;L431CB;L431CC;L431KB;L431KC;L431RB;L431RC;L431VC;L432KB;L432KC;L433CB;L433CC;L433RB;L433RC;L433VC;L442KC;L443CC;L443RC;L443VC;L451CC;L451CE;L451RC;L451RE;L451VC;L451VE;L452CC;L452CE;L452RC;L452RE;L452VC;L452VE;L462CE;L462RE;L462VE;L471QE;L471QG;L471RE;L471RG;L471VE;L471VG;L471ZE;L471ZG;L475RC;L475RE;L475RG;L475VC;L475VE;L475VG;L476JE;L476JG;L476ME;L476MG;L476QE;L476QG;L476RC;L476RE;L476RG;L476VC;L476VE;L476VG;L476ZE;L476ZG;L486JG;L486QG;L486RG;L486VG;L486ZG;L496AE;L496AG;L496QE;L496QG;L496RE;L496RG;L496VE;L496VG;L496ZE;L496ZG;L4A6AG;L4A6QG;L4A6RG;L4A6VG;L4A6ZG;L4P5AE;L4P5AG;L4P5CE;L4P5CG;L4P5QE;L4P5QG;L4P5RE;L4P5RG;L4P5VE;L4P5VG;L4P5ZE;L4P5ZG;L4Q5AG;L4Q5CG;L4Q5QG;L4Q5RG;L4Q5VG;L4Q5ZG;L4R5AG;L4R5AI;L4R5QG;L4R5QI;L4R5VG;L4R5VI;L4R5ZG;L4R5ZI;L4R7AI;L4R7VI;L4R7ZI;L4R9AG;L4R9AI;L4R9VG;L4R9VI;L4R9ZG;L4R9ZI;L4S5AI;L4S5QI;L4S5VI;L4S5ZI;L4S7AI;L4S7VI;L4S7ZI;L4S9AI;L4S9VI;L4S9ZI [build] -- STM_DEVICES is

atsju · 2021-07-23T15:12:25Z

cmake/stm32/devices.cmake

+            message(WARNING "Passed list ${ARGV0} invalid. Still setting STM_DEVICES variable")
+        endif()
+    endif()
+    if(ARGC MATCHES 2)


MATCHES is for regex. Here EQUAL may be more appropriate
https://cmake.org/cmake/help/latest/command/if.html#matches

atsju · 2021-07-23T15:14:25Z

cmake/stm32/devices.cmake

+        if(NOT ARGV1 STREQUAL "FAMILY")
+            message(WARNING "Unknown keyword: ${ARGV1}. Matching all devices..")
+        endif()


I think this can be handled by cmake_parse_arguments see https://cmake.org/cmake/help/latest/command/cmake_parse_arguments.html
<prefix>_KEYWORDS_MISSING_VALUES and <prefix>_UNPARSED_ARGUMENTS
I think it will be safer and avoid unexpected corner cases

cmake/stm32/devices.cmake

atsju · 2021-07-23T15:28:01Z

cmake/stm32/devices.cmake

+    set(ARG_OPTIONS "")
+    set(ARG_SINGLE "")
+    set(ARG_MULTIPLE FAMILY)
+    cmake_parse_arguments(PARSE_ARGV 0 ARG "${ARG_OPTIONS}" "${ARG_SINGLE}" "${ARG_MULTIPLE}")
+    # Some basic argument checks
+    if(ARGC MATCHES 1)
+        # FAMILY is the only keyword here
+        if(NOT ARGV STREQUAL "FAMILY")
+            message(WARNING "Unknown keyword: ${ARGV}. Matching all devices..")
+        endif()
+        if(NOT ARG_FAMILY)
+            message(STATUS "FAMILY keyword specified without any families. Matching all devices..")
+        endif()
    endif()


maybe all checks are done in stm32_get_devices_by_family.

In devices.cmake

robamu · 2021-07-24T10:13:02Z

I added a pretty printer as well. The tests appear to be problematic now though..

robamu · 2021-07-24T10:56:20Z

I did not realize the stm32_get_devices_by_family function is used in all the tests.. But I think I will just use the way you did it and set the expanded variable.

That took longer than expected. It would be great if you could run all your checks again. I hope we will be able to merge it soon

README.md

cmake/stm32/devices.cmake

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Hish15

I'm willing to do the PR, but lets stop adding commits to it first :D
Waiting the Go

atsju

@Hish15 Go
Maybe squash merge ?

Improved stm32_get_devices_by_family to print out the devices

robamu added 3 commits June 6, 2021 17:05

added print function

2b07232

added more information on function usage (cmake)

28cec71

readme fix

896a490

This was referenced Jun 8, 2021

Print device by family function #203

Closed

Print device by family function us-irs/stm32-cmake#11

Merged

atsju reviewed Jun 26, 2021

View reviewed changes

atsju mentioned this pull request Jun 26, 2021

Added example for h743zi board #206

Closed

atsju requested a review from Hish15 June 26, 2021 15:14

robamu added 2 commits June 28, 2021 10:01

added doc and warning if list is empty

a8eb114

better printout

1bed72e

robamu added 2 commits July 2, 2021 09:56

Merge remote-tracking branch 'upstream/master' into mueller/print-by-…

1823926

…family

Merge remote-tracking branch 'upstream/master' into mueller/print-by-…

436eeab

…family

atsju reviewed Jul 14, 2021

View reviewed changes

Hish15 reviewed Jul 14, 2021

View reviewed changes

Hish15 requested changes Jul 16, 2021

View reviewed changes

Update cmake/stm32/devices.cmake

7d75f95

Co-authored-by: Hish15 <pharamhector@gmail.com>

robamu changed the title ~~Print device by family function~~ WIP: Print device by family function Jul 16, 2021

updated STM32 utilities to get dev list

77b82c2

1. More checks 2. Multiple families can now be specified

robamu changed the title ~~WIP: Print device by family function~~ Print device by family function Jul 20, 2021

robamu added 4 commits July 20, 2021 16:19

removed core keyword for stm32_get_devices_by_family

d3b9bf9

Merge remote-tracking branch 'upstream/master' into mueller/print-by-…

14081eb

…family

including devices.cmake as well

0820291

README update

2b9e8e4

atsju requested changes Jul 23, 2021

View reviewed changes

robamu changed the title ~~Print device by family function~~ WIP: Print device by family function Jul 24, 2021

added pretty printer

6ce3f54

In devices.cmake

robamu added 2 commits July 24, 2021 12:18

test

e7d3577

maybe this fixes the issue?

5a263a6

robamu changed the title ~~WIP: Print device by family function~~ Print device by family function Jul 24, 2021

robamu changed the title ~~Print device by family function~~ WIP: Print device by family function Jul 24, 2021

robamu added 5 commits July 24, 2021 12:35

trying the non-expanded version again

eb4e24c

test2

b66eb58

test3

5b800fe

trying to get more output

4f3dc56

finally found the issue

8aed69d

setting expanded variable again

22358c6

robamu changed the title ~~WIP: Print device by family function~~ Print device by family function Jul 24, 2021

robamu mentioned this pull request Jul 24, 2021

update minimum cmake version to 3.16 #245

Merged

atsju reviewed Jul 25, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

cmake/stm32/devices.cmake Outdated Show resolved Hide resolved

cmake/stm32/devices.cmake Outdated Show resolved Hide resolved

atsju requested a review from Hish15 July 25, 2021 09:56

robamu and others added 3 commits July 25, 2021 12:25

Update cmake/stm32/devices.cmake

df3f206

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Update cmake/stm32/devices.cmake

216ff8d

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Update README.md

1a90d7a

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Hish15 reviewed Jul 25, 2021

View reviewed changes

atsju approved these changes Jul 26, 2021

View reviewed changes

atsju requested a review from Hish15 July 26, 2021 06:06

atsju assigned Hish15 Jul 27, 2021

Hish15 approved these changes Aug 1, 2021

View reviewed changes

Hish15 merged commit 70a5a0a into ObKo:master Aug 1, 2021

atsju mentioned this pull request Aug 1, 2021

CMSIS RTOS Feature #253

Merged

zisi pushed a commit to zisi/stm32-cmake that referenced this pull request Aug 2, 2021

Print device by family function (ObKo#205)

5718979

Improved stm32_get_devices_by_family to print out the devices

robamu deleted the mueller/print-by-family branch August 8, 2021 13:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print device by family function #205

Print device by family function #205

robamu commented Jun 8, 2021

atsju left a comment •

edited

Loading

robamu commented Jun 26, 2021 •

edited

Loading

robamu commented Jun 28, 2021

atsju Jul 14, 2021

robamu Jul 15, 2021

atsju Jul 15, 2021

robamu Jul 15, 2021 •

edited

Loading

atsju Jul 15, 2021

Hish15 left a comment

Hish15 Jul 16, 2021

atsju Jul 16, 2021

robamu commented Jul 16, 2021 •

edited

Loading

robamu commented Jul 20, 2021 •

edited

Loading

atsju commented Jul 20, 2021

robamu commented Jul 20, 2021

atsju left a comment

atsju Jul 23, 2021

atsju Jul 23, 2021

atsju Jul 23, 2021

atsju Jul 23, 2021

robamu commented Jul 24, 2021

robamu commented Jul 24, 2021 •

edited

Loading

Hish15 left a comment

atsju left a comment

		* `stm32_get_devices_by_family(DEVICES [FAMILY <family>])` - return into `DEVICES` all supported devices by family (or all devices if `<family>` is empty)
		* `stm32_print_devices_by_family([FAMILY <family>])` - Print all supported devices by family (or all devices if `<family>` is empty)

Print device by family function #205

Print device by family function #205

Conversation

robamu commented Jun 8, 2021

atsju left a comment • edited Loading

Choose a reason for hiding this comment

robamu commented Jun 26, 2021 • edited Loading

robamu commented Jun 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robamu Jul 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hish15 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robamu commented Jul 16, 2021 • edited Loading

robamu commented Jul 20, 2021 • edited Loading

atsju commented Jul 20, 2021

robamu commented Jul 20, 2021

atsju left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robamu commented Jul 24, 2021

robamu commented Jul 24, 2021 • edited Loading

Hish15 left a comment

Choose a reason for hiding this comment

atsju left a comment

Choose a reason for hiding this comment

atsju left a comment •

edited

Loading

robamu commented Jun 26, 2021 •

edited

Loading

robamu Jul 15, 2021 •

edited

Loading

robamu commented Jul 16, 2021 •

edited

Loading

robamu commented Jul 20, 2021 •

edited

Loading

robamu commented Jul 24, 2021 •

edited

Loading