nodemcu-firmware
nodemcu-firmware copied to clipboard
Add modules struct, bit, color_utils and sjson to luac.cross
Added for linux build and MSC. Excluded MinGW as I don't have an installation. But hope I did not break that build.
Partially Fixes #3229.
- [x] This PR is for the
dev
branch rather than formaster
. - [x] This PR is compliant with the other contributing guidelines as well (if not, please describe why).
- [x] I have thoroughly tested my contribution.
- [ ] The code changes are reflected in the documentation at
docs/*
.
<Description of and rationale behind this PR> Help creating tests and allow usage of known functionality on the host.
Sure it compiles. I just thought there might be defines which would default to something different later on. BTW the sjson.h and .c have many includes in common. Should I remove them and in which file
IMO, a module -- if it is implemented in a single file -- only needs a .h
header if it exports a publicly callable interface. If it doesn't then you have to question why it has one at all.
A header file should only include other headers if that are needed to compile a source that includes it. For example if it's prototypes use a type whose typedef
is in another file, though strictly they should use #include_once
, not that any do.
I don't know what #include_once means in the C realm. But all includes have guards against double inclusion.
It's a mindfart from my PHP days. :rofl:
Gregor, whilst I think on:
-
luac.cross
must be compilable and executable with theTEST=1
option. This enables the Lua Test Suite additions, and this also adds extra integrity checking on memory allocation. -
If we are going to add these then it make sense to do so optionally, e.g. have a
LUAC_EXTRA
option. -
I am currently doing a lot of rework of luac.cross to support #3272. Can we hold off on this PR until after this has landed?
Can we add pipe
to the list of modules available in luac.cross
?
Can we add
pipe
to the list of modules available inluac.cross
?
@nwf the HOST environment contains a minimal emulation of the subset of the platform interface sufficient to allow the Lua core and the small number of supported modules to build and to be callable within the luac.cross -e
execution environment. Pipe uses the task interface, so I would need to emulate this as well. This would require a bit of work. In essence, we would probably need to add a node.js-style task scheduler. I think that this counts as a separate task in itself that needs its own cost-benefit analysis.
Nathaniel, BTW, this isn't a -1 to your suggestion, more that it isn't a small change and so we should look at it in the wider context of perhaps providing a sensible subset NodeMCU
environment for host use.
Three questions about the implementation remain:
- Do we need to have this optional as Terry suggests?
- pipe currently is added without the possibility of the callback due to a lack of task.post Should we just do nothing or throw an error if it is used anyways (task.post is implemented empty for host builds)
- I changed luac.cross 5.1 as it passed the filename as parameter to a lua script which interferes with the convention to allow passing an optional NTest instance to a testfile. Is that OK? Its that way on 5.3 anyways.
How should I proceed?
- currently pixbuf is not added to the win build due to using ssize_t and dynamic sized arrays should we go without for now or fix it?
- Should I add pixbuf host tests in this PR?
Anybody want to vote for this PR It helps testing Lua programs on the host by adding several hardware unrelated modules to the luac.cross compiler which can also be used to execute Lua programs on the host. I am using this for years now and it speeds up dev in some cases.
Looks OK by me; if CI's happy, I'm happy. <3
Thanks guys. Happy to see this merged.