sprhawk / python-on-a-chip

Automatically exported from code.google.com/p/python-on-a-chip
Other
0 stars 0 forks source link

Consolidate HAVE_* platform feature definitions #88

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
platform/*/pmfeatures.h contains a bunch of definitions (HAVE_*) that 
configure the features of the VM.  But in order for the VM to work, the code 
images need to know which HAVE_* definitions exist.  This is done by setting 
similarly-named variable in pmImgCreator.py

It would be beneficial to not have to set the variables in pmImgCreator.py 
because each platform has different definitions.  Instead, it would be good 
to write a script that scans pmfeatures.h to generate config data to be used 
by pmImgCreator.

In other words, automate things so that the HAVE_* definitions/vars don't 
exist in 2 places and have to be in agreement.

Original issue reported on code.google.com by dwhall...@gmail.com on 20 May 2010 at 10:58

GoogleCodeExporter commented 9 years ago

Original comment by dwhall...@gmail.com on 20 May 2010 at 10:58

GoogleCodeExporter commented 9 years ago
set an environment variable called PLATFORM to the name of a directory in 
src/platform
and ensure that a file called pmfeatures.py exists in that directory.

Modify the make file to set the PLATFORM environment variable and export it.

Is there a better way to export the variable from the Makefile?

Do you want a single global file with the per platform file only over ridding 
as necessary?

Do you mind having to set an environment variable when running ipm, or should 
that be modified to take a command line option?

Generating the pmfeatures.h file should be achievable using a templated file.

Let me know and I can update the patch if you want.

Original comment by michae...@gmail.com on 7 Aug 2010 at 7:59

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by dwhall...@gmail.com on 12 Aug 2010 at 2:39

GoogleCodeExporter commented 9 years ago
Thanks, michaelInt for the input, but I've found a little bit simpler way to do 
this:

- Convert src/platform/<plat>/pmfeatures.h to a .py file containing simply a 
PM_FEATURES dict.
- Create a tool to read pmfeatures.py and create a corresponding .h when the 
platform is built.
- Edit pmImgCreator.py to remove PM_FEATURES and instead get it from the 
pmfeatures.py file.

Original comment by dwhall...@gmail.com on 3 Sep 2010 at 1:28

GoogleCodeExporter commented 9 years ago
Detailed list of changes:

- The PM_FEATURES dict was removed from src/tools/pmImgCreator.py;  Instead, a 
path to a file containing the PM_FEATURES dict must be passed as an argument 
when calling pmImgCreator.py.
- Since some "constant" values depended on some PM_FEATURES, such "constants" 
had to be relocated to the class constructor in pmImgCreator.py so they can be 
adjusted based on a command line argument.
- Since pmImgCreator.py requires a new argument, src/tools/ipm.py (which uses 
pmImgCreator.py) has to modified to provide this argument.  So ipm.py also 
requires the same path-to-pmfeatures argument on its command line.
- Nearly ALL of the makefiles have to be modified to provide the new arguments 
to pmImgCreator.py and ipm.py
- SConscript files for vm/, platform/stm32 and platform/windows were modified 
to provide the new arguments to pmImgCreator.py
- Feature doxy docs and dependency checks from the old pmfeatures.h files were 
consolidated in src/vm/pmFeatureDependencies.h

For each platform:
- All the HAVE_* definitions were converted to Python and placed in a new file 
src/platform/<platform>/pmfeatures.py
- Any other misc definition in pmfeatures.h was moved to a new file 
src/platform/<platform>/plat.h
- The platform pmfeatures.h file was removed from the repository
- If there was a patch file that modified pmImgCreator.py, it was removed from 
the repository
END FOR

- src/vm/plat.h was renamed to src/vm/plat_interface.h (to avoid name conflict 
with the new platform plat.h files
- src/tools/pmGenPmFeatures.py created to generate plat/pmfeatures.h from 
plat/pmfeatures.py  (all <platform>/Makefiles adjusted to do this)
- vm/pm.h list of includes was modified so plat.h has position near top, added 
plat_interface.h

Original comment by dwhall...@gmail.com on 6 Sep 2010 at 1:34

GoogleCodeExporter commented 9 years ago
Since you have already done the hard work of the builder that generates the 
PM_FEATURES dictionary from the .h file, why not generate pmImgCreator.py 
itself from the same script.

You would have "pmImgCreator.py.template" in tools folder with the PM_FEATURES 
dict replaced with special marker. Then all it would take is text replace this 
special marker to generate a new pmImgCreateor.py. You may want to set this as 
AlwaysBuild() in scons to be on the safe side.

Then there wouldn't be too many changes from how it is done today.

Original comment by eyob.b...@gmail.com on 6 Sep 2010 at 4:33

GoogleCodeExporter commented 9 years ago
That's a clever idea.  Using "AlwaysBuild()" wouldn't just be on the safe side, 
it would be a requirement.  And I would need to figure out the equivalent for 
the Makefiles.  And I would need to figure out how to make it work for the case 
when running ipm for a different target.

PS: pmGenPmFeatures.py works the reverse of what you describe: it uses .py 
input to generate a .h.  And it's a trivial script.

Original comment by dwhall...@gmail.com on 6 Sep 2010 at 5:06

GoogleCodeExporter commented 9 years ago
I find a little bit awkward that tools files are modified by the built process. 

1) distribution (&svn) wise : the file will be modified depending on the last 
built
2) mixed code & data ... blahblahblah ... no good
3) HAVE_* definitions still duplicated (but synchronized)

I'd really prefer the approach given in comment #5

Jumping back on proposition to use an environment variable, 
PYTHONPATH="src/platform/<plat>/" might be enough to export the platform; then 
'import pmfeatures' to retrieve the features dict ... pythonic, isn't it ?

Original comment by monnet.a...@gmail.com on 8 Sep 2010 at 2:10

GoogleCodeExporter commented 9 years ago
I've thought through the ideas presented in comment #6 and realize that there 
would still be problems when switching from one platform to another (which is 
common because of testing on desktop and then on embedded platform).

I'll be proceeding with what I presented (and have coded) in comment #5.  It's 
not ideal: the new solution may present new bugs; I can't test every platform 
(I don't have windows, stm32 or Teensy++).  But I think it will be better than 
the current system that is ALWAYS a gotcha for newcomers.

Original comment by dwhall...@gmail.com on 8 Sep 2010 at 2:43

GoogleCodeExporter commented 9 years ago
r593
Performed changes explained in Comment #5.

All tests pass.  Builds work for all platforms except for these which were 
untested: stm32, windows.

Mainlined directly

Original comment by dwhall...@gmail.com on 8 Sep 2010 at 9:32

GoogleCodeExporter commented 9 years ago
r594

- Updated docs/src/HowToPortPyMite.txt

Post mainline checkin.

Original comment by dwhall...@gmail.com on 8 Sep 2010 at 9:45

GoogleCodeExporter commented 9 years ago
Cool! I can't figure out though where to put platform specific definitions like 
PM_HEAP_SIZE, since now pmfeatures.h is automatically regenerated with every 
build

Original comment by zigli...@gmail.com on 16 Sep 2010 at 4:15

GoogleCodeExporter commented 9 years ago
Found:
- Any other misc definition in pmfeatures.h was moved to a new file 
src/platform/<platform>/plat.h

Original comment by zigli...@gmail.com on 16 Sep 2010 at 4:17