dbrgn / cube-parse

MIT License
8 stars 2 forks source link

Add support for parsing flash and ram sizes #5

Closed azerupi closed 3 years ago

azerupi commented 3 years ago

This PR should solve part of the problem for stm32-rs/stm32l0xx-hal#170

When generating the MCU features I get the output below for the two MCUs I gave as an example. Which seems to be correct.

mcu-STM32L051K8Ux = ["stm32l0x1", "ufqfpn32", "io-STM32L051", "eeprom-2048", "flash-64", "ram-8"]
mcu-STM32L071K8Ux = ["stm32l0x1", "ufqfpn32", "io-STM32L071", "eeprom-3072", "flash-64", "ram-20"]

About the implementation, I tried to make as little change as possible. I'm not sure the tuple in the hasmap is the cleanest way to do it. A better idea would probably be to merge the two Mcu structs. Let me know if you have a better idea for the implementation.

For the flash and ram features it generates the following:

# Features based on Flash size (in kbytes)
flash-128 = []
flash-16 = []
flash-192 = []
flash-32 = []
flash-64 = []
flash-8 = []

# Features based on RAM size (in kbytes)
ram-2 = []
ram-20 = []
ram-8 = []

I think it is quite manageable to handle this manually in the build.rs. But if needed we could generate the if/else wall automatically here too, would that be desired?

I just noticed flash and ram features are sorted alphabetically instead of numerically. I'm going to correct that before you merge this.

azerupi commented 3 years ago

I just noticed flash and ram features are sorted alphabetically instead of numerically. I'm going to correct that before you merge this.

Done

# Features based on Flash size (in kbytes)
flash-8 = []
flash-16 = []
flash-32 = []
flash-64 = []
flash-128 = []
flash-192 = []

# Features based on RAM size (in kbytes)
ram-2 = []
ram-8 = []
ram-20 = []
dbrgn commented 3 years ago

Oh, something else I noticed: You're taking the flash/ram sizes as &str, that's why you need to parse them as numbers for sorting.

Instead, maybe you could parse the numbers before even inserting them into the hashmap? This way, a regular sort() will do, and we are sure that only valid numbers are accepted (and something like n/a would fail). Take a look at the EEPROM handling, there we're also using numbers, not strings.

azerupi commented 3 years ago

Everything should be addressed. I ran rustfmt but due to the GitHub first contribution restrictions I can't see if CI passes, but I would assume it does.

azerupi commented 3 years ago

@dbrgn ping :slightly_smiling_face:

dbrgn commented 3 years ago

@azerupi sorry, I wanted to review/merge tonight, but didn't find the time (and I'm not at home over the weekend). I'll try to do it next week. Feel free to ping me, in case I forget (I mostly use notification e-mails as reminders).

azerupi commented 3 years ago

No problem at all, thanks! :slightly_smiling_face: