cpu/fe310/gpio: use gpio_irq feature#10007
Conversation
|
The binary size doesn't change with this PR, regardless if I compile with or without |
The hifive1 is on my desk at FU. I won't be there for a while, maybe you could use that for testing? |
|
Actually, this problem was caused because I was building with docker (see #10001 (comment)). I'll retry with setting the variables within the Makefile instead. |
miri64
left a comment
There was a problem hiding this comment.
It still grows for some reason by 8 bytes compared to master without periph_gpio_irq, but I was able to compare the build sizes and diffs like I did in e.g. #10002:
As far as I can tell (and checked with
colordiff -u <(git show | grep "^-" | sed 's/^-//g') <(git show | grep "^+" | sed 's/^+//g')) this just moves andifdefs some code around. After a local rebase to include #9981 I also was able to build this withtests/periph_gpioboth withperiph_gpio_irqmanually added or removed toFEATURES_OPTIONAL. […]
As with #9997, this PR needed another level of diffing, because the move wasn't that obvious, but I checked this as well:
colordiff -u \ <(diff -u <(git show | grep "^-" | sed 's/^-//g') \ <(git show | grep "^+" | sed 's/^+//g') | grep "^-" | sed 's/^-//g') \ <(diff -u <(git show | grep "^-" | sed 's/^-//g') \ <(git show | grep "^+" | sed 's/^+//g') | grep "^+" | sed 's/^+//g')
|
When I execute the make command directly within docker, it works as expected and the image size reduces when Is that something I should look at? You said it seems to be specific for esp8266 platform. |
I asked @cladmi about it and it is a known, but not easy (or even practicable) to fix problem. Basically, the docker container would need to get every possible variables handed via the environment filtered from irrelevand variables. This sounded to me rather like a nice to have, than something sensible to implement or maintain, so I didn't open an issue for that. But anyway... this is completely unrelated from this PR |
|
It's easy to fix on a per variable basic, it means adding it in here: Line 27 in 4e92c2a But what I meant it is more a global issue of knowing what should be exported globally than just adding one variable there. A solution to make it flexible could be to use |
The per variable basis was the thing that sounded to me as "something [that is not] sensible to implement or maintain" ;-). I'm not sure your proposed solution is really one. You would need to know it the same way you would need to know that some variables are not provided to the docker container's environment. |
|
You would need to know indeed, but currently even if you know you can't do it from the command line. It would just allow setting some non handled variables. And even if you don't know, you could basically always add it to |
See #9992