Open dhalbert opened 1 year ago
@dhalbert Thank you for letting me know. I'm currently checking if we can change the assert in NuttX to also work in CircuitPython. But this is not a quick solution. This will take time.
However, in the meantime I noticed that the Spresense SDK is built with the NDEBUG flag: https://github.com/sonydevworld/spresense-exported-sdk/blob/c12296c5ffbd0779e630a653c76a78558b463271/nuttx/.config#L58
But the configuration was not used in CircuitPython. Below is a patch that fixes this:
diff --git a/ports/cxd56/Makefile b/ports/cxd56/Makefile
index 238edb96e4..81b1b171e3 100644
--- a/ports/cxd56/Makefile
+++ b/ports/cxd56/Makefile
@@ -23,6 +23,7 @@
# THE SOFTWARE.
include ../../py/circuitpy_mkenv.mk
+include ./spresense-exported-sdk/nuttx/.config
CROSS_COMPILE = arm-none-eabi-
@@ -91,6 +92,10 @@ CFLAGS += \
-fdata-sections \
-Wall \
+ifeq ($(CONFIG_NDEBUG),y)
+ CFLAGS += -DNDEBUG
+endif
+
OPTIMIZATION_FLAGS ?= -O2 -fno-inline-functions
# option to override compiler optimization level, set in boards/$(BOARD)/mpconfigboard.mk
At this point you should be able to build CircuitPython without any problem with Spresense. However, when the NDEBUG flag is enabled, assert will do nothing.
Maybe CircuitPython could have its own assert implementation? What do you think about this?
Thanks for your comments. Right now we simply removed the troublesome assert, and that may be all we need to do. In the long run perhaps we could talk to the nuttx people about the issue. It does seem to be a rare problem, though. We will also see about NDEBUG, as you mentioned.
@kamtom480: a question for you:
The implementation of
assert()
in spresense comes from nuttx.assert()
is defined asASSERT()
whenNDEBUG
is not defined. See https://github.com/sonydevworld/spresense-nuttx/blob/new-master/include/assert.h:This version of
assert()
cannot be used in an expression context, such as a comma expression. It can only be used in statement context.We are merging recent changes from upstream MicroPython into CircuitPython, and there are several places where MicroPython uses
assert()
in a comma expression here: https://github.com/micropython/micropython/blob/05cb1406ad1b421a238faf763e19f4119f5f6bb2/py/obj.h#L921-L926This doesn't work with the statement-only
do ... while(0)
version ofassert()
above, and so we can't build the spresense port in in our merge branch right now.man 3 assert
saysassert()
has the signaturevoid assert(scalar expression);
, though it might be implemented as a macro.The other versions of
assert()
in the other CircuitPython and MicroPython can be used in expressions.I am not sure how to fix this: the particular macro above is tricky and hard to rewrite with the nuttx assert. The nuttx assert could be modified, but that is a change in that submodule. Or a new version of
assert()
could be introduced to substitute for the nuttx version.What do you think? Thanks.
We don't have a merge PR yet, but @tannewt and I are working in https://github.com/dhalbert/circuitpython/tree/v1.20-merge