From f4ace4a36dd15ae768638ecda4a420ee9adbdae2 Mon Sep 17 00:00:00 2001 From: Thomas Clark Date: Tue, 5 Aug 2014 11:48:47 -0400 Subject: [PATCH] Cleaned up C++ compiler warnings All C++ projects now build without warnings with -Wall and -Wextra Change-Id: Idb6cf8b78274a30453e98c1e8edabcfb2a7fffb6 --- CMakeLists.txt | 2 +- hal/lib/Athena/Accelerometer.cpp | 1 + hal/lib/Athena/Analog.cpp | 4 +- hal/lib/Athena/Digital.cpp | 10 +- hal/lib/Athena/ctre/CtreCanNode.cpp | 3 +- wpilibc/wpilibC++/lib/DigitalInput.cpp | 2 +- wpilibc/wpilibC++/lib/PIDController.cpp | 104 +++++++++--------- wpilibc/wpilibC++/lib/SPI.cpp | 8 -- .../src/CANJaguarTest.cpp | 6 +- .../src/gtest/src/gtest.cc | 2 + 10 files changed, 66 insertions(+), 76 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 53a30153ae..904133b882 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 2.8) project(All-WPILib) set(CMAKE_BUILD_TYPE Debug) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wformat=2 -Wextra -Wno-unused-parameter -fPIC") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wformat=2 -Wall -Wextra -Wno-unused-parameter -fPIC") SET(CMAKE_SKIP_BUILD_RPATH TRUE) file(GLOB_RECURSE NI_LIBS ni-libraries/*.so*) diff --git a/hal/lib/Athena/Accelerometer.cpp b/hal/lib/Athena/Accelerometer.cpp index 28bf2a55ff..0895c9c084 100644 --- a/hal/lib/Athena/Accelerometer.cpp +++ b/hal/lib/Athena/Accelerometer.cpp @@ -167,6 +167,7 @@ static double unpackAxis(int16_t raw) { case kRange_2G: return raw / 1024.0; case kRange_4G: return raw / 512.0; case kRange_8G: return raw / 256.0; + default: return 0.0; } } diff --git a/hal/lib/Athena/Analog.cpp b/hal/lib/Athena/Analog.cpp index 6265b8e2f0..02a39b9706 100644 --- a/hal/lib/Athena/Analog.cpp +++ b/hal/lib/Athena/Analog.cpp @@ -102,7 +102,7 @@ bool checkAnalogModule(uint8_t module) { * @return Analog channel is valid */ bool checkAnalogInputChannel(uint32_t pin) { - if (pin >= 0 && pin < kAnalogInputPins) + if (pin < kAnalogInputPins) return true; return false; } @@ -115,7 +115,7 @@ bool checkAnalogInputChannel(uint32_t pin) { * @return Analog channel is valid */ bool checkAnalogOutputChannel(uint32_t pin) { - if (pin >= 0 && pin < kAnalogOutputPins) + if (pin < kAnalogOutputPins) return true; return false; } diff --git a/hal/lib/Athena/Digital.cpp b/hal/lib/Athena/Digital.cpp index 90efa7b4e2..51ce738ccd 100644 --- a/hal/lib/Athena/Digital.cpp +++ b/hal/lib/Athena/Digital.cpp @@ -1215,7 +1215,7 @@ void spiClose(uint8_t port) { */ void spiSetSpeed(uint8_t port, uint32_t speed) { Synchronized sync(spiGetSemaphore(port)); - int retVal = spilib_setspeed(spiGetHandle(port), speed); + spilib_setspeed(spiGetHandle(port), speed); } /** @@ -1228,7 +1228,7 @@ void spiSetSpeed(uint8_t port, uint32_t speed) { */ void spiSetOpts(uint8_t port, int msb_first, int sample_on_trailing, int clk_idle_high) { Synchronized sync(spiGetSemaphore(port)); - int retVal = spilib_setopts(spiGetHandle(port), msb_first, sample_on_trailing, clk_idle_high); + spilib_setopts(spiGetHandle(port), msb_first, sample_on_trailing, clk_idle_high); } /** @@ -1276,22 +1276,16 @@ int32_t spiGetHandle(uint8_t port){ switch(port){ case 0: return m_spiCS0Handle; - break; case 1: return m_spiCS1Handle; - break; case 2: return m_spiCS2Handle; - break; case 3: return m_spiCS3Handle; - break; case 4: return m_spiMXPHandle; - break; default: return 0; - break; } } diff --git a/hal/lib/Athena/ctre/CtreCanNode.cpp b/hal/lib/Athena/ctre/CtreCanNode.cpp index 3b66d37b22..30a31a024a 100644 --- a/hal/lib/Athena/ctre/CtreCanNode.cpp +++ b/hal/lib/Athena/ctre/CtreCanNode.cpp @@ -20,7 +20,7 @@ void CtreCanNode::RegisterTx(uint32_t arbId, uint32_t periodMs) { int32_t status = 0; - txJob_t job = {0}; + txJob_t job; job.arbId = arbId; job.periodMs = periodMs; _txJobs[arbId] = job; @@ -96,4 +96,3 @@ void CtreCanNode::FlushTx(uint32_t arbId) iter->second.periodMs, &status); } - diff --git a/wpilibc/wpilibC++/lib/DigitalInput.cpp b/wpilibc/wpilibC++/lib/DigitalInput.cpp index 2081816d79..9eb0fb6ef7 100644 --- a/wpilibc/wpilibC++/lib/DigitalInput.cpp +++ b/wpilibc/wpilibC++/lib/DigitalInput.cpp @@ -28,7 +28,7 @@ void DigitalInput::InitDigitalInput(uint32_t channel) m_channel = channel; int32_t status = 0; - bool allocated = allocateDIO(m_digital_ports[channel], true, &status); + allocateDIO(m_digital_ports[channel], true, &status); wpi_setErrorWithContext(status, getHALErrorMessage(status)); HALReport(HALUsageReporting::kResourceType_DigitalInput, channel); diff --git a/wpilibc/wpilibC++/lib/PIDController.cpp b/wpilibc/wpilibC++/lib/PIDController.cpp index f4688d2e9d..55f0e58c1a 100644 --- a/wpilibc/wpilibC++/lib/PIDController.cpp +++ b/wpilibc/wpilibC++/lib/PIDController.cpp @@ -65,7 +65,7 @@ void PIDController::Initialize(float Kp, float Ki, float Kd, float Kf, m_I = Ki; m_D = Kd; m_F = Kf; - + m_maximumOutput = 1.0; m_minimumOutput = -1.0; @@ -86,17 +86,17 @@ void PIDController::Initialize(float Kp, float Ki, float Kd, float Kf, m_pidInput = source; m_pidOutput = output; m_period = period; - + pthread_mutexattr_t mutexattr; pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_RECURSIVE); pthread_mutex_init(&m_mutex, &mutexattr); - - pthread_create(&m_controlLoop, NULL, PIDController::CallCalculate, this); + + pthread_create(&m_controlLoop, NULL, PIDController::CallCalculate, this); static int32_t instances = 0; instances++; HALReport(HALUsageReporting::kResourceType_PIDController, instances); - + m_toleranceType = kNoTolerance; } @@ -110,7 +110,7 @@ PIDController::~PIDController() pthread_mutex_lock(&m_mutex); m_destruct = true; pthread_mutex_unlock(&m_mutex); - + pthread_join(m_controlLoop, NULL); } @@ -125,41 +125,43 @@ void *PIDController::CallCalculate(void *data) { PIDController *controller = (PIDController*) data; int destruct = 0; - + while(!destruct) { controller->Calculate(); - + /* End the calculation loop when the PIDController gets destructed */ pthread_mutex_lock(&controller->m_mutex); destruct = controller->m_destruct; pthread_mutex_unlock(&controller->m_mutex); - + Wait(controller->m_period); } + + return NULL; } /** * Read the input, calculate the output accordingly, and write to the output. * This should only be called by the Notifier indirectly through CallCalculate * and is created during initialization. - */ + */ void PIDController::Calculate() { bool enabled; PIDSource *pidInput; - + if(m_pidInput == 0) return; if(m_pidOutput == 0) return; - + pthread_mutex_lock(&m_mutex); enabled = m_enabled; pidInput = m_pidInput; pthread_mutex_unlock(&m_mutex); - + if(enabled) { pthread_mutex_lock(&m_mutex); - + float input = pidInput->PIDGet(); float result; @@ -180,7 +182,7 @@ void PIDController::Calculate() } } } - + if(m_I != 0) { double potentialIGain = (m_totalError + m_error) * m_I; @@ -205,9 +207,9 @@ void PIDController::Calculate() pidOutput = m_pidOutput; result = m_result; - + pidOutput->PIDWrite(result); - + pthread_mutex_unlock(&m_mutex); } } @@ -266,11 +268,11 @@ void PIDController::SetPID(float p, float i, float d, float f) float PIDController::GetP() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_P; + temp = m_P; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -281,11 +283,11 @@ float PIDController::GetP() float PIDController::GetI() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_I; + temp = m_I; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -296,11 +298,11 @@ float PIDController::GetI() float PIDController::GetD() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_D; + temp = m_D; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -311,11 +313,11 @@ float PIDController::GetD() float PIDController::GetF() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_F; + temp = m_F; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -327,11 +329,11 @@ float PIDController::GetF() float PIDController::Get() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_result; + temp = m_result; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -351,7 +353,7 @@ void PIDController::SetContinuous(bool continuous) /** * Sets the maximum and minimum values expected from the input. - * + * * @param minimumInput the minimum value expected from the input * @param maximumInput the maximum value expected from the output */ @@ -359,7 +361,7 @@ void PIDController::SetInputRange(float minimumInput, float maximumInput) { pthread_mutex_lock(&m_mutex); m_minimumInput = minimumInput; - m_maximumInput = maximumInput; + m_maximumInput = maximumInput; pthread_mutex_unlock(&m_mutex); SetSetpoint(m_setpoint); @@ -367,7 +369,7 @@ void PIDController::SetInputRange(float minimumInput, float maximumInput) /** * Sets the minimum and maximum values to write. - * + * * @param minimumOutput the minimum value to write to the output * @param maximumOutput the maximum value to write to the output */ @@ -400,7 +402,7 @@ void PIDController::SetSetpoint(float setpoint) m_setpoint = setpoint; } pthread_mutex_unlock(&m_mutex); - + if (m_table != NULL) { m_table->PutNumber("setpoint", m_setpoint); } @@ -413,11 +415,11 @@ void PIDController::SetSetpoint(float setpoint) float PIDController::GetSetpoint() { float temp; - + pthread_mutex_lock(&m_mutex); - temp = m_setpoint; + temp = m_setpoint; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -428,11 +430,11 @@ float PIDController::GetSetpoint() float PIDController::GetError() { float error; - + pthread_mutex_lock(&m_mutex); error = m_setpoint - m_pidInput->PIDGet(); pthread_mutex_unlock(&m_mutex); - + return error; } @@ -485,7 +487,7 @@ void PIDController::SetAbsoluteTolerance(float absTolerance) bool PIDController::OnTarget() { bool temp; - + pthread_mutex_lock(&m_mutex); switch (m_toleranceType) { case kPercentTolerance: @@ -499,7 +501,7 @@ bool PIDController::OnTarget() temp = false; } pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -508,10 +510,10 @@ bool PIDController::OnTarget() */ void PIDController::Enable() { - pthread_mutex_lock(&m_mutex); + pthread_mutex_lock(&m_mutex); m_enabled = true; pthread_mutex_unlock(&m_mutex); - + if (m_table != NULL) { m_table->PutBoolean("enabled", true); } @@ -526,7 +528,7 @@ void PIDController::Disable() m_pidOutput->PIDWrite(0); m_enabled = false; pthread_mutex_unlock(&m_mutex); - + if (m_table != NULL) { m_table->PutBoolean("enabled", false); } @@ -538,11 +540,11 @@ void PIDController::Disable() bool PIDController::IsEnabled() { bool temp; - + pthread_mutex_lock(&m_mutex); - temp = m_enabled; + temp = m_enabled; pthread_mutex_unlock(&m_mutex); - + return temp; } @@ -600,7 +602,7 @@ void PIDController::ValueChanged(ITable* source, const std::string& key, EntryVa } void PIDController::UpdateTable() { - + } void PIDController::StartLiveWindowMode() { @@ -608,5 +610,5 @@ void PIDController::StartLiveWindowMode() { } void PIDController::StopLiveWindowMode() { - + } diff --git a/wpilibc/wpilibC++/lib/SPI.cpp b/wpilibc/wpilibC++/lib/SPI.cpp index 2b9d291c87..e2abe4f0c7 100644 --- a/wpilibc/wpilibC++/lib/SPI.cpp +++ b/wpilibc/wpilibC++/lib/SPI.cpp @@ -45,7 +45,6 @@ SPI::~SPI() */ void SPI::SetClockRate(double hz) { - int32_t retVal = 0; spiSetSpeed(m_port, hz); } @@ -55,7 +54,6 @@ void SPI::SetClockRate(double hz) */ void SPI::SetMSBFirst() { - int32_t retVal = 0; m_msbFirst = true; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -66,7 +64,6 @@ void SPI::SetMSBFirst() */ void SPI::SetLSBFirst() { - int32_t retVal = 0; m_msbFirst = false; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -77,7 +74,6 @@ void SPI::SetLSBFirst() */ void SPI::SetSampleDataOnFalling() { - int32_t retVal = 0; m_sampleOnTrailing = true; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -88,7 +84,6 @@ void SPI::SetSampleDataOnFalling() */ void SPI::SetSampleDataOnRising() { - int32_t retVal = 0; m_sampleOnTrailing = false; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -99,7 +94,6 @@ void SPI::SetSampleDataOnRising() */ void SPI::SetClockActiveLow() { - int32_t retVal = 0; m_clk_idle_high = true; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -110,7 +104,6 @@ void SPI::SetClockActiveLow() */ void SPI::SetClockActiveHigh() { - int32_t retVal = 0; m_clk_idle_high = false; spiSetOpts(m_port, (int) m_msbFirst, (int) m_sampleOnTrailing, (int) m_clk_idle_high); } @@ -188,4 +181,3 @@ int32_t SPI::Transaction(uint8_t* dataToSend, uint8_t* dataReceived, uint8_t siz retVal = spiTransaction(m_port, dataToSend, dataReceived, size); return retVal; } - diff --git a/wpilibc/wpilibC++IntegrationTests/src/CANJaguarTest.cpp b/wpilibc/wpilibC++IntegrationTests/src/CANJaguarTest.cpp index 03df7d077b..6efb5c32c6 100644 --- a/wpilibc/wpilibC++IntegrationTests/src/CANJaguarTest.cpp +++ b/wpilibc/wpilibC++IntegrationTests/src/CANJaguarTest.cpp @@ -256,7 +256,7 @@ TEST_F(CANJaguarTest, VoltageModeWorks) { float setpoints[] = { M_PI, 8.0f, -10.0f }; - for(int i = 0; i < sizeof(setpoints)/sizeof(setpoints[0]); i++) { + for(unsigned int i = 0; i < sizeof(setpoints)/sizeof(setpoints[0]); i++) { float setpoint = setpoints[i]; SetJaguar(kMotorTime, setpoint); @@ -312,13 +312,13 @@ TEST_F(CANJaguarTest, CurrentModeWorks) { float setpoints[] = { 1.6f, 2.0f, -1.6f }; - for(int i = 0; i < sizeof(setpoints)/sizeof(setpoints[0]); i++) { + for(unsigned int i = 0; i < sizeof(setpoints)/sizeof(setpoints[0]); i++) { float setpoint = setpoints[i]; float expectedCurrent = std::abs(setpoints[i]); /* It should get to each setpoint within 10 seconds */ for(int j = 0; j < 10; j++) { - SetJaguar(1.0, setpoints[i]); + SetJaguar(1.0, setpoint); if(std::abs(m_jaguar->GetOutputCurrent() - expectedCurrent) <= kCurrentTolerance) { break; diff --git a/wpilibc/wpilibC++IntegrationTests/src/gtest/src/gtest.cc b/wpilibc/wpilibC++IntegrationTests/src/gtest/src/gtest.cc index 408e6f2c3c..24c573c5a6 100644 --- a/wpilibc/wpilibC++IntegrationTests/src/gtest/src/gtest.cc +++ b/wpilibc/wpilibC++IntegrationTests/src/gtest/src/gtest.cc @@ -50,6 +50,8 @@ #include #include +#pragma GCC diagnostic ignored "-Wmissing-field-initializers" + #if GTEST_OS_LINUX // TODO(kenton@google.com): Use autoconf to detect availability of