Skip to content

Commit baa7acf

Browse files
authored
fix input type deduction for default conversions (#59)
1 parent 4e65041 commit baa7acf

File tree

4 files changed

+21
-9
lines changed

4 files changed

+21
-9
lines changed

config_utilities/include/config_utilities/internal/visitor.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,15 @@ struct Visitor {
169169
// Dispatch populating field input info from conversions.
170170
template <typename Conversion,
171171
typename ConfigT,
172+
typename IntermediateT,
172173
typename std::enable_if<!hasFieldInputInfo<Conversion>() || isConfig<ConfigT>(), bool>::type = true>
173-
static void getFieldInputInfo(const std::string& field_name);
174+
static void getFieldInputInfo(const IntermediateT& intermediate, const std::string& field_name);
174175

175176
template <typename Conversion,
176177
typename ConfigT,
178+
typename IntermediateT,
177179
typename std::enable_if<hasFieldInputInfo<Conversion>() && !isConfig<ConfigT>(), bool>::type = true>
178-
static void getFieldInputInfo(const std::string& field_name);
180+
static void getFieldInputInfo(const IntermediateT& intermediate, const std::string& field_name);
179181

180182
// Computes the default values for all fields in the meta data. This assumes that the meta data is already created,
181183
// and the meta data was created from ConfigT.

config_utilities/include/config_utilities/internal/visitor_impl.hpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,13 @@ void Visitor::visitField(T& field, const std::string& field_name, const std::str
211211
auto intermediate = Conversion::toIntermediate(field, error);
212212
if (!error.empty()) {
213213
visitor.data.errors.emplace_back(new Warning(field_name, error));
214-
error.clear();
215214
}
216215

217216
Visitor::visitField(intermediate, field_name, unit);
218217

219218
// Get type information if requested.
220219
if (visitor.mode == Visitor::Mode::kGetInfo) {
221-
Visitor::getFieldInputInfo<Conversion, T>(field_name);
220+
Visitor::getFieldInputInfo<Conversion, T>(intermediate, field_name);
222221
}
223222
}
224223
}
@@ -457,8 +456,9 @@ void Visitor::getDefaultValues(const ConfigT& config, MetaData& data) {
457456
// intentional no-op
458457
template <typename Conversion,
459458
typename ConfigT,
459+
typename IntermediateT,
460460
typename std::enable_if<!hasFieldInputInfo<Conversion>() || isConfig<ConfigT>(), bool>::type>
461-
void Visitor::getFieldInputInfo(const std::string& field_name) {
461+
void Visitor::getFieldInputInfo(const IntermediateT&, const std::string& field_name) {
462462
static_assert(!isConfig<ConfigT>() || !hasFieldInputInfo<Conversion>(),
463463
"Config types (with declare_config) cannot have field input information!");
464464

@@ -471,14 +471,18 @@ void Visitor::getFieldInputInfo(const std::string& field_name) {
471471
visitor.data.errors.emplace_back(
472472
new Warning(field_name, "Invalid parsing state! Field info should already exist!"));
473473
} else {
474-
visitor.data.field_infos.back().input_info.reset(); // clear field input info for underlying intermediate type
474+
// Default: Create the field input info of the intermediate type.
475+
auto input_info = createFieldInputInfo<IntermediateT>();
476+
auto& info = visitor.data.field_infos.back();
477+
info.input_info = FieldInputInfo::merge(input_info, info.input_info);
475478
}
476479
}
477480

478481
template <typename Conversion,
479482
typename ConfigT,
483+
typename IntermediateT,
480484
typename std::enable_if<hasFieldInputInfo<Conversion>() && !isConfig<ConfigT>(), bool>::type>
481-
void Visitor::getFieldInputInfo(const std::string& field_name) {
485+
void Visitor::getFieldInputInfo(const IntermediateT&, const std::string& field_name) {
482486
auto input_info = Conversion::getFieldInputInfo();
483487

484488
auto& visitor = Visitor::instance();

config_utilities/test/tests/conversions.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,10 @@ TEST(Conversions, FieldInputInfo) {
243243
ConversionStruct without_info;
244244
data = internal::Visitor::getInfo(without_info);
245245
EXPECT_EQ(data.field_infos.size(), 2);
246-
EXPECT_FALSE(data.field_infos[0].input_info);
247-
EXPECT_FALSE(data.field_infos[1].input_info);
246+
EXPECT_TRUE(data.field_infos[0].input_info);
247+
EXPECT_TRUE(data.field_infos[1].input_info);
248+
EXPECT_EQ(data.field_infos[0].input_info->type, internal::FieldInputInfo::Type::kInt); // num_threads
249+
EXPECT_EQ(data.field_infos[1].input_info->type, internal::FieldInputInfo::Type::kString); // some_character
248250
}
249251

250252
TEST(Conversions, ConversionDeclareConfigDispatch) {

config_utilities_ros/config_utilities_ros/gui/dynamic_config_gui.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,10 @@ def parse_rec(config, indent, prefix):
314314
prefix_str = "".join([f"{p}{NS_SEP}" for p in prefix])
315315
for field in config["fields"]:
316316
if field["type"] == "field":
317+
# Default the input info to just strings if no details are present.
318+
if "input_info" not in field:
319+
field["input_info"] = {"type": "yaml"}
320+
317321
# Data for each field (leaves of the config).
318322
field["id"] = f"{prefix_str}{field['name']}"
319323
field["indent"] = indent

0 commit comments

Comments
 (0)