Merge "Annotate missing break statement"
diff --git a/debian/clapack.BUILD b/debian/clapack.BUILD
index 9648bab..8d805c3 100644
--- a/debian/clapack.BUILD
+++ b/debian/clapack.BUILD
@@ -1,6 +1,7 @@
licenses(["notice"])
load("@//tools/build_rules:fortran.bzl", "f2c_copts")
+load("@//tools/build_rules:select.bzl", "compiler_select")
genrule(
name = "create_sysdep1",
@@ -280,7 +281,6 @@
"-Wno-sign-compare",
"-Wno-cast-qual",
"-Wno-cast-align",
- "-Wno-self-assign",
# Some files don't #include system headers when they should. sysdep1.h
# messes with feature test macros, so it always has to come first.
@@ -292,7 +292,14 @@
# Don't mangle the names of all the BLAS symbols, because slicot needs to
# call them directly.
"-DNO_BLAS_WRAP",
- ],
+ ] + compiler_select({
+ "clang": [
+ "-Wno-self-assign",
+ ],
+ "gcc": [
+ "-Wno-discarded-qualifiers",
+ ],
+ }),
includes = [
"F2CLIBS/libf2c",
"INCLUDE",
diff --git a/debian/slycot.BUILD b/debian/slycot.BUILD
index 368b87d..7463173 100644
--- a/debian/slycot.BUILD
+++ b/debian/slycot.BUILD
@@ -2,6 +2,7 @@
licenses(["restricted"])
load("@//tools/build_rules:fortran.bzl", "f2c_library")
+load("@//tools/build_rules:select.bzl", "compiler_select")
# We can't create _wrapper.so in the slycot folder, and can't move it.
# The best way I found to do this is to modify _wrapper.pyf to instead generate
@@ -135,7 +136,13 @@
# This gets triggered because it doesn't realize xerbla doesn't return.
# TODO(Brian): Try and get __attribute__((noreturn)) on xerbla somehow.
"-Wno-uninitialized",
- ],
+ ] + compiler_select({
+ "clang": [
+ ],
+ "gcc": [
+ "-Wno-discarded-qualifiers",
+ ],
+ }),
visibility = ["//visibility:public"],
deps = [
"@clapack",
diff --git a/frc971/control_loops/drivetrain/polydrivetrain.h b/frc971/control_loops/drivetrain/polydrivetrain.h
index d16e8a1..b93c1de 100644
--- a/frc971/control_loops/drivetrain/polydrivetrain.h
+++ b/frc971/control_loops/drivetrain/polydrivetrain.h
@@ -57,10 +57,16 @@
// Returns the current estimated velocity in m/s.
Scalar velocity() const {
- return (loop_->mutable_X_hat()(0) + loop_->mutable_X_hat()(1)) / 2.0;
+ return (loop_->mutable_X_hat()(0) + loop_->mutable_X_hat()(1)) * kHalf;
}
private:
+ static constexpr Scalar kZero = static_cast<Scalar>(0.0);
+ static constexpr Scalar kHalf = static_cast<Scalar>(0.5);
+ static constexpr Scalar kOne = static_cast<Scalar>(1.0);
+ static constexpr Scalar kTwo = static_cast<Scalar>(2.0);
+ static constexpr Scalar kTwelve = static_cast<Scalar>(12.0);
+
StateFeedbackLoop<7, 2, 4, Scalar> *kf_;
const ::aos::controls::HVPolytope<2, 4, 4, Scalar> U_Poly_;
@@ -127,14 +133,16 @@
const constants::ShifterHallEffect &hall_effect, Scalar shifter_position,
Scalar velocity, Gear gear) {
const Scalar high_gear_speed =
- velocity / dt_config_.high_gear_ratio / dt_config_.wheel_radius;
+ velocity /
+ static_cast<Scalar>(dt_config_.high_gear_ratio / dt_config_.wheel_radius);
const Scalar low_gear_speed =
- velocity / dt_config_.low_gear_ratio / dt_config_.wheel_radius;
+ velocity /
+ static_cast<Scalar>(dt_config_.low_gear_ratio / dt_config_.wheel_radius);
- if (shifter_position < hall_effect.clear_low) {
+ if (shifter_position < static_cast<Scalar>(hall_effect.clear_low)) {
// We're in low gear, so return speed for that gear.
return low_gear_speed;
- } else if (shifter_position > hall_effect.clear_high) {
+ } else if (shifter_position > static_cast<Scalar>(hall_effect.clear_high)) {
// We're in high gear, so return speed for that gear.
return high_gear_speed;
}
@@ -195,11 +203,12 @@
const bool highgear = goal.highgear;
// Apply a sin function that's scaled to make it feel better.
- const Scalar angular_range = M_PI_2 * dt_config_.wheel_non_linearity;
+ const Scalar angular_range =
+ static_cast<Scalar>(M_PI_2) * dt_config_.wheel_non_linearity;
wheel_ = sin(angular_range * wheel) / sin(angular_range);
wheel_ = sin(angular_range * wheel_) / sin(angular_range);
- wheel_ = 2.0 * wheel - wheel_;
+ wheel_ = kTwo * wheel - wheel_;
quickturn_ = quickturn;
if (quickturn_) {
@@ -208,12 +217,12 @@
wheel_ *= dt_config_.wheel_multiplier;
}
- static const Scalar kThrottleDeadband = 0.05;
+ static constexpr Scalar kThrottleDeadband = static_cast<Scalar>(0.05);
if (::std::abs(throttle) < kThrottleDeadband) {
throttle_ = 0;
} else {
throttle_ = copysign(
- (::std::abs(throttle) - kThrottleDeadband) / (1.0 - kThrottleDeadband),
+ (::std::abs(throttle) - kThrottleDeadband) / (kOne - kThrottleDeadband),
throttle);
}
@@ -252,10 +261,10 @@
const Scalar high_min_FF_sum = FF_high.col(0).sum();
const Scalar adjusted_ff_voltage =
- ::aos::Clip(throttle * 12.0 * min_FF_sum / high_min_FF_sum, -12.0, 12.0);
+ ::aos::Clip(throttle * kTwelve * min_FF_sum / high_min_FF_sum, -kTwelve, kTwelve);
return (adjusted_ff_voltage +
- ttrust_ * min_K_sum * (loop_->X_hat(0, 0) + loop_->X_hat(1, 0)) /
- 2.0) /
+ ttrust_ * min_K_sum * (loop_->X_hat(0, 0) + loop_->X_hat(1, 0)) *
+ kHalf) /
(ttrust_ * min_K_sum + min_FF_sum);
}
@@ -278,7 +287,7 @@
const Scalar high_min_FF_sum = FF_high.col(0).sum();
const Scalar adjusted_ff_voltage =
- ::aos::Clip(12.0 * min_FF_sum / high_min_FF_sum, -12.0, 12.0);
+ ::aos::Clip(kTwelve * min_FF_sum / high_min_FF_sum, -kTwelve, kTwelve);
return adjusted_ff_voltage / min_FF_sum;
}
@@ -307,7 +316,7 @@
// and that the plant is the same on the left and right.
const Scalar fvel = FilterVelocity(throttle_);
- const Scalar sign_svel = wheel_ * ((fvel > 0.0) ? 1.0 : -1.0);
+ const Scalar sign_svel = wheel_ * ((fvel > kZero) ? kOne : -kOne);
Scalar steering_velocity;
if (quickturn_) {
steering_velocity = wheel_ * MaxVelocity();
@@ -327,7 +336,7 @@
// K * R = w
Eigen::Matrix<Scalar, 1, 2> equality_k;
equality_k << 1 + sign_svel, -(1 - sign_svel);
- const Scalar equality_w = 0.0;
+ const Scalar equality_w = kZero;
// Construct a constraint on R by manipulating the constraint on U
::aos::controls::HVPolytope<2, 4, 4, Scalar> R_poly_hv(
@@ -358,10 +367,10 @@
}
// Housekeeping: set the shifting logging values to zero, because we're not shifting
- left_motor_speed_ = 0.0;
- right_motor_speed_ = 0.0;
- current_left_velocity_ = 0.0;
- current_right_velocity_ = 0.0;
+ left_motor_speed_ = kZero;
+ right_motor_speed_ = kZero;
+ current_left_velocity_ = kZero;
+ current_right_velocity_ = kZero;
} else {
current_left_velocity_ =
(position_.left_encoder - last_position_.left_encoder) / dt_config_.dt;
@@ -384,17 +393,17 @@
R_left(0, 0) = left_motor_speed_;
R_right(0, 0) = right_motor_speed_;
- const Scalar wiggle =
- (static_cast<Scalar>((counter_ % 30) / 15) - 0.5) * 8.0;
+ const Scalar wiggle = (static_cast<Scalar>((counter_ % 30) / 15) - kHalf) *
+ static_cast<Scalar>(8.0);
loop_->mutable_U(0, 0) = ::aos::Clip(
(R_left / dt_config_.v)(0, 0) + (IsInGear(left_gear_) ? 0 : wiggle),
- -12.0, 12.0);
+ -kTwelve, kTwelve);
loop_->mutable_U(1, 0) = ::aos::Clip(
(R_right / dt_config_.v)(0, 0) + (IsInGear(right_gear_) ? 0 : wiggle),
- -12.0, 12.0);
+ -kTwelve, kTwelve);
#ifdef __linux__
- loop_->mutable_U() *= 12.0 / ::aos::robot_state->voltage_battery;
+ loop_->mutable_U() *= kTwelve / ::aos::robot_state->voltage_battery;
#endif // __linux__
}
}
diff --git a/tools/build_rules/BUILD b/tools/build_rules/BUILD
index e69de29..f96fb3a 100644
--- a/tools/build_rules/BUILD
+++ b/tools/build_rules/BUILD
@@ -0,0 +1,5 @@
+sh_binary(
+ name = "quiet_success",
+ srcs = ["quiet_success.sh"],
+ visibility = ["//visibility:public"],
+)
diff --git a/tools/build_rules/fortran.bzl b/tools/build_rules/fortran.bzl
index e8b25c1..cbd4447 100644
--- a/tools/build_rules/fortran.bzl
+++ b/tools/build_rules/fortran.bzl
@@ -1,4 +1,4 @@
-load('@//tools/build_rules:select.bzl', 'compiler_select')
+load("@//tools/build_rules:select.bzl", "compiler_select")
def _single_fortran_object_impl(ctx):
toolchain_cflags = (ctx.fragments.cpp.compiler_options([]) +
@@ -47,22 +47,24 @@
'pic_o': fortran_file_base + '.pic.o',
}
-
_single_fortran_object = rule(
- implementation = _single_fortran_object_impl,
- attrs = {
- 'src': attr.label(single_file=True, allow_files=FileType(['.f'])),
- 'cc_libs': attr.label_list(providers=['cc']),
- # TODO(Brian): Replace this with something more fine-grained from the
- # configuration fragment or something.
- '_cc_toolchain': attr.label(
- default = Label('@//tools/cpp:toolchain'),
- ),
- },
- outputs = _define_fortran_output,
- fragments = [
- 'cpp',
- ],
+ attrs = {
+ "src": attr.label(
+ single_file = True,
+ allow_files = FileType([".f"]),
+ ),
+ "cc_libs": attr.label_list(providers = ["cc"]),
+ # TODO(Brian): Replace this with something more fine-grained from the
+ # configuration fragment or something.
+ "_cc_toolchain": attr.label(
+ default = Label("@//tools/cpp:toolchain"),
+ ),
+ },
+ fragments = [
+ "cpp",
+ ],
+ outputs = _define_fortran_output,
+ implementation = _single_fortran_object_impl,
)
def fortran_library(name, srcs, deps = [], visibility = None):
@@ -95,28 +97,28 @@
)
f2c_copts = compiler_select({
- 'clang': [
- '-Wno-incompatible-pointer-types-discards-qualifiers',
- # Clang appears to be a bit over-eager about this and the comma operator.
- '-Wno-sometimes-uninitialized',
- ],
- 'gcc': [
- # TODO(Brian): Remove this once we can actually disable all the warnings.
- # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43245 isn't fixed in our
- # roborio toolchain yet, so we can't for now.
- '-Wno-error',
- ],
+ "clang": [
+ "-Wno-incompatible-pointer-types-discards-qualifiers",
+ # Clang appears to be a bit over-eager about this and the comma operator.
+ "-Wno-sometimes-uninitialized",
+ ],
+ "gcc": [
+ # TODO(Brian): Remove this once we can actually disable all the warnings.
+ # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43245 isn't fixed in our
+ # roborio toolchain yet, so we can't for now.
+ "-Wno-error",
+ ],
}) + [
- # f2c appears to know what it's doing without adding extra ().
- '-Wno-parentheses',
-
- '-Wno-unused-parameter',
- '-Wno-missing-field-initializers',
- '-Wno-unused-variable',
+ # f2c appears to know what it's doing without adding extra ().
+ "-Wno-parentheses",
+ "-Wno-unused-parameter",
+ "-Wno-missing-field-initializers",
+ "-Wno-unused-variable",
]
-'''Copts to use when compiling f2c-generated files.
-This is useful when building externally-f2ced files.'''
+"""Copts to use when compiling f2c-generated files.
+
+This is useful when building externally-f2ced files."""
def f2c_library(name, srcs, copts = [], **kwargs):
'''Converts Fortran code to C and then compiles it.
@@ -141,8 +143,14 @@
outs = c_srcs,
tools = [
'@f2c',
+ '@//tools/build_rules:quiet_success',
],
- cmd = '$(location @f2c) -d$(@D)/%s $(SRCS)' % ('/'.join(out_dir),),
+ cmd = ' '.join([
+ '$(location @//tools/build_rules:quiet_success)',
+ '$(location @f2c)',
+ '-d$(@D)/%s' % ('/'.join(out_dir),),
+ '$(SRCS)',
+ ]),
)
native.cc_library(
name = name,
diff --git a/tools/build_rules/quiet_success.sh b/tools/build_rules/quiet_success.sh
new file mode 100755
index 0000000..6020cef
--- /dev/null
+++ b/tools/build_rules/quiet_success.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+# This program hides all the output on stderr from the called command, unless it
+# fails, in which case all the output is printed at the end.
+
+set -e
+set -u
+
+readonly STDERR_FILE="$(mktemp)"
+
+if ! "$@" 2>"${STDERR_FILE}" ; then
+ cat "${STDERR_FILE}"
+ exit 1
+fi