Skip to content

debug build improvements #7591

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

debug build improvements #7591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ports/raspberrypi/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ CFLAGS += $(OPTIMIZATION_FLAGS)
CFLAGS += $(CFLAGS_CYW43)
#Debugging/Optimization
ifeq ($(DEBUG), 1)
CFLAGS += -ggdb3 -O3
CFLAGS += -ggdb3 -O1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would usually use -Og, but you said that didn't work. if -O1 works, let's leave it and add a comment about what was tried and what didn't work.

# No LTO because we may place some functions in RAM instead of flash.
else
CFLAGS += -DNDEBUG
Expand Down
1 change: 1 addition & 0 deletions ports/raspberrypi/boards/raspberry_pi_pico_w/mpconfigboard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#define CIRCUITPY_DIGITALIO_HAVE_INVALID_DRIVE_MODE (1)

#define MICROPY_HW_LED_STATUS (&pin_CYW0)
#define MICROPY_PY_COLLECTIONS_DEQUE (1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be promoted up to mpconfigport.h. It should be on for all but the smallest builds.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deque is used by wifi, and not all rp2 boards have wifi, so that's why I put it at the board level.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #6474; we generally want deque on; only turning it off on the smallest builds.


#define CIRCUITPY_BOARD_I2C (1)
#define CIRCUITPY_BOARD_I2C_PIN {{.scl = &pin_GPIO5, .sda = &pin_GPIO4}}
1 change: 1 addition & 0 deletions ports/raspberrypi/boards/raspberry_pi_pico_w/mpconfigboard.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ CIRCUITPY_SSL = 1
CIRCUITPY_SSL_MBEDTLS = 1
CIRCUITPY_HASHLIB = 1
CIRCUITPY_WEB_WORKFLOW = 1
CIRCUITPY_OS_GETENV = 1
CIRCUITPY_MDNS = 1
CIRCUITPY_SOCKETPOOL = 1
CIRCUITPY_WIFI = 1
Expand Down
9 changes: 5 additions & 4 deletions ports/raspberrypi/common-hal/wifi/Monitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
* THE SOFTWARE.
*/

#ifndef MICROPY_INCLUDED_ESPRESSIF_COMMON_HAL_WIFI_MONITOR_H
#define MICROPY_INCLUDED_ESPRESSIF_COMMON_HAL_WIFI_MONITOR_H
#pragma once

#if !CIRCUITPY_WIFI
#error wifi not in build
#endif
Comment on lines +30 to +32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would rather have this kind of checking in the .mk files. If you do provide these messages, I would make them say CIRCUITPY_WIFI = 0, but this file is being included for clarity. But the corresponding C files should not be being compiled. If they are not, then there would be link errors, which is how things would be caught.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this is just an optimization for the build to fail at compile time instead of link time, because I'm impatient waiting for builds.
A typical scenario is I'm working on some module, and I sneak in a dependency on another module by including its header file. This compiles fine, but if the other module is not in the build config I happen to be using, I will get a link error. I think it would be hard to check this in .mk files since .mk files don't know what header files C files are including.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using make -j12 etc.? I'd guess yes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a bigger computer. 😅

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having #error all around the code is a +1 from me.
Linking can take 20s on x64 (-j12) and 4 minutes on my jacked (16% overclock) pi400.


#include "py/obj.h"

Expand All @@ -36,5 +39,3 @@ typedef struct {
size_t lost;
size_t queue_length;
} wifi_monitor_obj_t;

#endif // MICROPY_INCLUDED_ESPRESSIF_COMMON_HAL_WIFI_MONITOR_H
4 changes: 4 additions & 0 deletions ports/raspberrypi/common-hal/wifi/Network.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

#pragma once

#if !CIRCUITPY_WIFI
#error wifi not in build
#endif

#include "py/obj.h"

#include "pico/cyw43_arch.h"
Expand Down
4 changes: 4 additions & 0 deletions ports/raspberrypi/common-hal/wifi/Radio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

#pragma once

#if !CIRCUITPY_WIFI
#error wifi not in build
#endif

#include "py/obj.h"

#include "shared-bindings/wifi/ScannedNetworks.h"
Expand Down
4 changes: 4 additions & 0 deletions ports/raspberrypi/common-hal/wifi/ScannedNetworks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@

#pragma once

#if !CIRCUITPY_WIFI
#error wifi not in build
#endif

#include <stdint.h>

#include "py/obj.h"
Expand Down
4 changes: 4 additions & 0 deletions ports/raspberrypi/common-hal/wifi/__init__.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

#pragma once

#if !CIRCUITPY_WIFI
#error wifi not in build
#endif

#include "py/obj.h"
#include "py/mpconfig.h"
#include "lwip/ip_addr.h"
Expand Down
4 changes: 4 additions & 0 deletions shared-module/bitbangio/I2C.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
#ifndef MICROPY_INCLUDED_SHARED_MODULE_BITBANGIO_I2C_H
#define MICROPY_INCLUDED_SHARED_MODULE_BITBANGIO_I2C_H

#if !CIRCUITPY_BITBANGIO
#error bitbangio not in build
#endif

#include "common-hal/digitalio/DigitalInOut.h"

#include "py/obj.h"
Expand Down
4 changes: 4 additions & 0 deletions shared-module/bitbangio/SPI.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
#ifndef MICROPY_INCLUDED_SHARED_MODULE_BITBANGIO_SPI_H
#define MICROPY_INCLUDED_SHARED_MODULE_BITBANGIO_SPI_H

#if (!CIRCUITPY_BITBANGIO) && (!CIRCUITPY_BUSIO_SPI)
#error bitbangio not in build
#endif

#include "common-hal/digitalio/DigitalInOut.h"

#include "py/obj.h"
Expand Down
4 changes: 4 additions & 0 deletions shared-module/os/__init__.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

#pragma once

#if !CIRCUITPY_OS_GETENV
#error os_getenv not in build
#endif

typedef enum {
GETENV_OK = 0,
GETENV_ERR_OPEN,
Expand Down
7 changes: 0 additions & 7 deletions supervisor/shared/web_workflow/web_workflow.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,9 @@
#include "shared-bindings/socketpool/Socket.h"
#include "shared-bindings/socketpool/SocketPool.h"

#if CIRCUITPY_WIFI
#include "shared-bindings/wifi/__init__.h"
#endif

#if CIRCUITPY_OS_GETENV
#include "shared-module/os/__init__.h"
#endif
Comment on lines -63 to -69
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense, because supervisor/supervisor.mk only compiles this file if CIRCUITPY_WEB_WORKFLOW = 1


enum request_state {
STATE_METHOD,
Expand Down Expand Up @@ -254,8 +250,6 @@ void supervisor_web_workflow_status(void) {
#endif

void supervisor_start_web_workflow(void) {
#if CIRCUITPY_WEB_WORKFLOW && CIRCUITPY_WIFI && CIRCUITPY_OS_GETENV

// Skip starting the workflow if we're not starting from power on or reset.
const mcu_reset_reason_t reset_reason = common_hal_mcu_processor_get_reset_reason();
if (reset_reason != RESET_REASON_POWER_ON &&
Expand Down Expand Up @@ -353,7 +347,6 @@ void supervisor_start_web_workflow(void) {
_api_password[0] = ':';
_base64_in_place(_api_password, strlen(_api_password), sizeof(_api_password) - 1);
}
#endif
}

void web_workflow_send_raw(socketpool_socket_obj_t *socket, const uint8_t *buf, int len) {
Expand Down