-
-
Notifications
You must be signed in to change notification settings - Fork 63
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
Support for FlexPRET platform #2262
Conversation
…add support for board target property.
@lhstrh / @erlingrj I'm having an issue with the baudrate target property. Specifically this line, which sets the default baudrate for a platform if the user does not specify any: lingua-franca/core/src/main/java/org/lflang/target/property/PlatformProperty.java Line 43 in f840daa
When there is a default baudrate, I'm not sure if I'll be able to determine whether the user tried to set the property or if it is just a default value. (If there is a way to do this, please let me know; I haven't programmed in Java until now, so I'm struggling to understand everything going on.) What I want to do for FlexPRET is not allow the user to set it, because this is something the FlexPRET SDK controls entirely. Maybe it would be possible to provide different default values depending on what platform is used? Or just set the default value of baudrate to 0 and have Arduino set it to 9600 somewhere else. |
core/src/main/java/org/lflang/target/property/PlatformProperty.java
Outdated
Show resolved
Hide resolved
The way it's currently set up, you can detect whether a property as a whole has been set using |
I guess it would solve my problem if the default is 9600 for Arduino and 0 for all other platforms, because 0 would in that case mean "not set". I guess the more robust solution would be having the |
…Util.java that contains a function to flash an executable to FlexPRET FPGA.
core/src/main/java/org/lflang/target/property/PlatformProperty.java
Outdated
Show resolved
Hide resolved
…not print anything during execution, which it should
Currently I'm struggling a bit with code size issues. When I compile a test program, e.g.,
(This is with FlexPRET using 256 kB for instruction SPM and data SPM, which is unrealistic but I'm using it to be able to compile the programs. A more realistic choice is 64 kB for both.) I have checked what takes up code size with FlexPRET has its own standalone I have found two possible methods to remove these functions.
The best option, of course, would be to figure out where in the LF source code the function gets pulled in from and see if we can filter this out for embedded systems. I found this method, but I ended up just tracking within newlib. I tried to do 2. on the same program, and got promising results:
I'm not quite sure why this reduces code size by more than 10 kB (the size of the function), but probably other code becomes redundant and does not get linked in either, I guess. I think both these options are kind of bad because they might cause headaches down the road. But they might be necessary. Do you have any thoughts @erlingrj? |
I dont fully understand. Do we have our own, fully-featured printf implementation for FlexPRET? Then we should make sure that this is what is being used for the functions in |
I think the problem is that I assume But until we have a |
The tzset probably comes from the call to |
Ah ok, that's where But for this PR it would maybe be a bit much to sanitize all of reactor-c from these function calls? Because I assume it's a big task to find all functions like |
Yes, this would only affect the FlexPRET target. It would be great if you made a Github issue for this problem and linked to it in a comment where you apply the wrap workaround. |
Is this a fair description of the issue? lf-lang/reactor-c#418 (comment) |
I have a few questions: (@erlingrj / @lhstrh)
Once I've solved these issues I think what's left is only:
|
I would say that if CI passes and you have made an effort to search it down, then we should merge it and fix any errors if they arise later on.
Since we have an emulator of FlexPRET we could run CI on it also. However, I don't think those tests belong here in the lingua-franca repo. I think we should just build a couple of HelloWorlds, maybe maybe run a few of them on the emulator. But it is not trivial, see next comment.
I don't think you can continuously monitor the output of commands, you only get it once the command terminates. I did a workaround for this in a bash script for running Zephyr tests on a QEMU emulator, since the QEMU emulator does not terminate. You will find it under .github/scripts I believe. It is a bash script that monitors the output and terminates if certain patterns are found. I don't think it is worth to do this for FlexPRET. The reason is that it is a little brittle, and FlexPRET has a big stack of dependencies and if some test for some reason fails, it is more likely to be a problem in the CPU design or the SDK than in LFC or reactor-c.
Great! |
… does not support [Y/n] interactive prompts in LFCommand.
This looks good now, with the only caveat that there are some tests that are "expected to pass" as part of the merge requirements but have now been moved. We can bypass the branch protection once we're ready to merge and then adjust the GitHub configuration. |
Thank you for fixing my typo! :) lf-lang/lf-lang.github.io#254 is now ready for review as well :) I think the only things left are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks very good. Just a few minor suggestions
@lhstrh I tried to transfer ownership of https://github.com/magnmaeh/lf-flexpret-template but I need access to create public repos on lf-lang to do that. |
@magnmaeh: sent you an invite to join the org! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT, modulo some nitpicks.
The failure in TS is tracked here #2293, I think we should merge even with this failure and this must be solved independently. The last issue is that zephyr and arduino tests appear as required but are not run: I don't really see why, those workflows are not references anywhere except in the c-embedded.yml (which runs) and in the reactor-c CI |
It's configured in the GitHub settings. This needs to be changed because the workflows have moved around. I'll take care of this. |
This PR adds support for the FlexPRET platform. In addition, it adds an
Option<T>
on the PlatformOptions type so we know whether a specific key/value pair was set by the user or just had its default value.Depends on lf-lang/reactor-c#412