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 support for parsing flash and ram sizes #5

Merged
merged 3 commits into from
Jul 30, 2021
Merged

Conversation

azerupi
Copy link

@azerupi azerupi commented Jun 10, 2021

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
Copy link
Author

azerupi commented Jun 10, 2021

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 = []

Copy link
Owner

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Hi @azerupi, this looks great!

Can you run cargo fmt to make CI happy?

@dbrgn
Copy link
Owner

dbrgn commented Jun 23, 2021

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
Copy link
Author

azerupi commented Jun 24, 2021

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
Copy link
Author

azerupi commented Jul 13, 2021

@dbrgn ping 🙂

@dbrgn
Copy link
Owner

dbrgn commented Jul 16, 2021

@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).

Copy link
Owner

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

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

Thank you @azerupi, and sorry for the long delay in reviewing!

@dbrgn dbrgn merged commit 2c71164 into dbrgn:master Jul 30, 2021
@azerupi
Copy link
Author

azerupi commented Jul 30, 2021

No problem at all, thanks! 🙂

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.

2 participants