-
-
Notifications
You must be signed in to change notification settings - Fork 175
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.
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
Review ESP32 compiler options to enable warnings as errors #754
Review ESP32 compiler options to enable warnings as errors #754
Conversation
…ompiler errors. Signed-off-by: Matthias Jentsch <[email protected]>
…ompiler errors. Signed-off-by: Matthias Jentsch <[email protected]>
Hi @MatthiasJentsch, I'm nanoFramework bot. A human will be reviewing it shortly. 😉 |
src/CLR/Diagnostics/Info.cpp
Outdated
@@ -226,6 +226,7 @@ int CLR_Debug::PrintfV( const char *format, va_list arg ) | |||
size_t iBuffer = MAXSTRLEN(buffer); | |||
|
|||
bool fRes = CLR_SafeSprintfV( szBuffer, iBuffer, format, arg ); | |||
(void)fRes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is still required after the PR with the missing ESP32 defines?! ASSERT should be available after that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. That's not needed anymore. I've corrected that.
@@ -117,8 +116,8 @@ HRESULT Library_win_dev_pwm_native_Windows_Devices_Pwm_PwmPin::NativeInit___VOID | |||
// get a pointer to the managed object instance and check that it's not NULL | |||
CLR_RT_HeapBlock* pThis = stack.This(); FAULT_ON_NULL(pThis); | |||
|
|||
int timerId = (int)(pThis[ FIELD___pwmTimer ].NumericByRef().u4); | |||
PwmPulsePolarity polarity = (PwmPulsePolarity)(pThis[ FIELD___polarity ].NumericByRef().u4); | |||
//int timerId = (int)(pThis[ FIELD___pwmTimer ].NumericByRef().u4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not required at all? if not better remove it instead of commenting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove it
@@ -131,7 +130,7 @@ HRESULT Library_win_dev_pwm_native_Windows_Devices_Pwm_PwmPin::NativeInit___VOID | |||
NANOCLR_CHECK_HRESULT( ConfigureAndStart(pThis, true, true) ); | |||
|
|||
// Get speed mode based on Timer used | |||
ledc_mode_t speed_mode = GetSpeedMode(timerId); | |||
//ledc_mode_t speed_mode = GetSpeedMode(timerId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove it
@@ -348,7 +355,7 @@ HRESULT Library_win_dev_serial_native_Windows_Devices_SerialCommunication_Serial | |||
uart_port_t uart_num = (uart_port_t)(pThis[ FIELD___portIndex ].NumericByRef().s4 - 1); | |||
|
|||
// Wait for 1 sec for data to be sent | |||
esp_err_t esp_err = uart_wait_tx_done( uart_num, (TickType_t) 1000 / portTICK_PERIOD_MS); | |||
/*esp_err_t esp_err =*/ uart_wait_tx_done( uart_num, (TickType_t) 1000 / portTICK_PERIOD_MS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't better to check for the return?
if not, better remove this too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check the return value
@@ -11,30 +11,30 @@ HRESULT Library_win_dev_spi_native_Windows_Devices_Spi_SpiBusInfo::get_MaxClockF | |||
{ | |||
NANOCLR_HEADER(); | |||
{ | |||
CLR_RT_HeapBlock* pArg = &(stack.Arg1()); | |||
//CLR_RT_HeapBlock* pArg = &(stack.Arg1()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove the unnecessary code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove the code
} | ||
|
||
HRESULT Library_win_dev_spi_native_Windows_Devices_Spi_SpiBusInfo::get_MinClockFrequency___I4( CLR_RT_StackFrame& stack ) | ||
{ | ||
NANOCLR_HEADER(); | ||
{ | ||
CLR_RT_HeapBlock* pArg = &(stack.Arg1()); | ||
//CLR_RT_HeapBlock* pArg = &(stack.Arg1()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove the code
@@ -335,7 +337,7 @@ HRESULT Library_win_dev_spi_native_Windows_Devices_Spi_SpiDevice::NativeInit___V | |||
{ | |||
NANOCLR_HEADER(); | |||
|
|||
spi_device_interface_config_t dev_config; | |||
//spi_device_interface_config_t dev_config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove the code
@@ -13,7 +13,7 @@ extern bool CLR_Messaging_ProcessPayload(WP_Message* msg); | |||
|
|||
|
|||
// Initialize to a packet sequence number impossible to encounter | |||
static uint32_t lastPacketSequence = 0x00FEFFFF; | |||
//static uint32_t lastPacketSequence = 0x00FEFFFF; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this shouldn't be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what to do here.
Signed-off-by: Matthias Jentsch <[email protected]>
Signed-off-by: Matthias Jentsch <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Added the compiler options -Wall -Wextra -Werror and removed all the -Wno-error... options for the ESP32 target.
Motivation and Context
How Has This Been Tested?
With the changes the ESP32 target compiles without errors
Types of changes
Checklist:
Signed-off-by: Matthias Jentsch [email protected]