Skip to content

Improve Log Messages in GPIO HAL #9011

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

Merged
merged 4 commits into from
Dec 19, 2023
Merged

Improve Log Messages in GPIO HAL #9011

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ed2f40
Improve Log Messages
SuGlider Dec 17, 2023
7facaa1
uses pin# in log messages
SuGlider Dec 18, 2023
a32278e
Uses IO # in messages
SuGlider Dec 19, 2023
65df2b7
Merge branch 'master' into gpio_hal_log
SuGlider Dec 19, 2023
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
12 changes: 6 additions & 6 deletions cores/esp32/esp32-hal-gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode)
#endif

if (pin >= SOC_GPIO_PIN_COUNT) {
log_e("Invalid pin selected");
log_e("Invalid pin (%i) selected", pin);
return;
}

if(perimanGetPinBus(pin, ESP32_BUS_TYPE_GPIO) == NULL){
perimanSetBusDeinit(ESP32_BUS_TYPE_GPIO, gpioDetachBus);
if(!perimanClearPinBus(pin)){
log_e("Deinit of previous bus failed");
log_e("Deinit of previous bus from GPIO %i failed", pin);
Copy link
Member

Choose a reason for hiding this comment

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

Pin is not set as GPIO yet, so better to use "pin" instead of "GPIO".

Copy link
Member

Choose a reason for hiding this comment

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

Pin: usually the physical pin number on a chip or module
GPIO: usually the hardware IO+IOBUS number inside the chip. Do not apply for special purpose IOs
IO: usually wider sense, which includes GPIO and special purpose IOs

BUT because we are special and have IO mode = GPIO and all our IOs can be GPIOs, let's use IO from now on for when we want to specify something on the hardware level that might not be in GPIO mode.

If we are in pinMode or going through the matrix, then we are in GPIO mode. If we are using something analog (ADC, DAC, Touch, etc.) we are in RTC mode. Else we are in special mode (I'm pretty sure we don't currently use it, but UART0 starts that way, Flash/PSRAM are that way also and if IO numbers match SPI, we could use it there too for better 80MHz support)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have changed the message in this and other places. Let me know if you agree.

return;
}
}
Expand Down Expand Up @@ -140,7 +140,7 @@ extern void ARDUINO_ISR_ATTR __pinMode(uint8_t pin, uint8_t mode)
}
if(gpio_config(&conf) != ESP_OK)
{
log_e("GPIO config failed");
log_e("GPIO %i config failed", pin);
return;
}
if(perimanGetPinBus(pin, ESP32_BUS_TYPE_GPIO) == NULL){
Expand All @@ -164,7 +164,7 @@ extern void ARDUINO_ISR_ATTR __digitalWrite(uint8_t pin, uint8_t val)
if(perimanGetPinBus(pin, ESP32_BUS_TYPE_GPIO) != NULL){
gpio_set_level((gpio_num_t)pin, val);
} else {
log_e("Pin is not set as GPIO.");
log_e("Pin %i is not set as GPIO.", pin);
}
}

Expand All @@ -174,7 +174,7 @@ extern int ARDUINO_ISR_ATTR __digitalRead(uint8_t pin)
return gpio_get_level((gpio_num_t)pin);
}
else {
log_e("Pin is not set as GPIO.");
log_e("Pin %i is not set as GPIO.", pin);
return 0;
}
}
Expand Down Expand Up @@ -204,7 +204,7 @@ extern void __attachInterruptFunctionalArg(uint8_t pin, voidFuncPtrArg userFunc,
interrupt_initialized = (err == ESP_OK) || (err == ESP_ERR_INVALID_STATE);
}
if(!interrupt_initialized) {
log_e("GPIO ISR Service Failed To Start");
log_e("GPIO %i ISR Service Failed To Start", pin);
return;
}

Expand Down