Skip to content
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

Improvements to binary and string variables #50

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions Test/FMI3/fmi3_import_start_arrays_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ TEST_CASE("Binary array") {
fmi3_import_free(xml);
}


TEST_CASE("Binary scalar") {
const char* xmldir = FMI3_TEST_XML_DIR "/arrays/valid";
fmi3_import_t* xml = fmi3_testutil_parse_xml(xmldir);
REQUIRE(xml != nullptr);

SECTION("Test binary scalar variable") {
fmi3_import_variable_t* v = fmi3_import_get_variable_by_name(xml, "binary_var");
fmi3_binary_t start = fmi3_import_get_binary_variable_start(fmi3_import_get_variable_as_binary(v));
REQUIRE(start[0] == 0x3cU);
REQUIRE(start[1] == 0x3fU);
}
fmi3_import_free(xml);
}

TEST_CASE("Test array parsing and verify retrieved start values are as expected"){
jm_callbacks cb;
fmi_import_context_t *context;
Expand Down
4 changes: 2 additions & 2 deletions Test/FMI3/fmi3_import_variable_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ TEST_CASE("Invalid Binary variable - non-hexadecimal char first in byte tuple",
const char* xmldir = FMI3_TEST_XML_DIR "/variable_test/invalid/binaryStart1";

fmi3_import_t* xml = fmi3_testutil_parse_xml(xmldir);
REQUIRE(xml != nullptr);
REQUIRE(xml != nullptr); /* TODO how do we want this to work? */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only remaining action is to figure out how we want to handle this scenario, currently the tests fail on this branch, but the tests only fail here.


const char* errMsg = fmi3_import_get_last_error(xml);
REQUIRE(strcmp(errMsg, "String is not hexadecimal: gf") == 0);
Expand All @@ -332,7 +332,7 @@ TEST_CASE("Invalid Binary variable - non-hexadecimal char second in byte tuple",
const char* xmldir = FMI3_TEST_XML_DIR "/variable_test/invalid/binaryStart2";

fmi3_import_t* xml = fmi3_testutil_parse_xml(xmldir);
REQUIRE(xml != nullptr);
REQUIRE(xml != nullptr); /* TODO how do we want this to work? */
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only remaining action is to figure out how we want to handle this scenario, currently the tests fail on this branch, but the tests only fail here.


const char* errMsg = fmi3_import_get_last_error(xml);
REQUIRE(strcmp(errMsg, "String is not hexadecimal: FG") == 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

<CoSimulation modelIdentifier="VariableTypes"/>
<ModelVariables>
<Binary name="binary_array" valueReference="122" maxSize="28" initial="exact" description="Testing binary arrays">
<Binary name="binary_array" valueReference="125" maxSize="28" initial="exact" description="Testing binary arrays">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

125 was an arbitrary change I did to simplify debugging (since there are other tests with vr 122)

<Dimension start="3"/>
<Start value="0011BBff029eE4Cd"/>
<Start value="3c3f"/>
Expand Down
5 changes: 1 addition & 4 deletions Test/FMI3/parser_test_xmls/arrays/valid/modelDescription.xml
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,8 @@
<Start value="A sTring value"/>
</String>

<Binary name="binary_array" valueReference="122" maxSize="28" initial="exact" description="Testing binary arrays">
<Dimension start="3"/>
<Start value="0011BBff029eE4Cd"/>
<Binary name="binary_var" valueReference="122" maxSize="28" initial="exact" description="Testing binary scalar">
<Start value="3c3f"/>
<Start value="E4Cd"/>
</Binary>

<!-- variables used as dimension specifiers -->
Expand Down
8 changes: 3 additions & 5 deletions src/XML/src/FMI3/fmi3_xml_type_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,14 @@ typedef struct fmi3_xml_int_variable_start_t {

typedef struct fmi3_xml_variable_start_string_t {
fmi3_xml_variable_type_base_t super;
jm_vector(jm_voidp) stringStartValues;
char start[1];
jm_vector(jm_voidp) values;
} fmi3_xml_string_variable_start_t;

typedef struct fmi3_xml_binary_variable_start_t {
fmi3_xml_variable_type_base_t super;
jm_vector(jm_voidp) binaryStartValues;
jm_vector(size_t) binaryStartValuesSize;
size_t nStart;
fmi3_uint8_t start[1]; /* NOTE: Can be longer than 1. Memory can be allocated outside of struct boundary. */
jm_vector(jm_voidp) values;
jm_vector(size_t) valuesSize;
} fmi3_xml_binary_variable_start_t;

// ----------------------------------------------------------------------------
Expand Down
111 changes: 40 additions & 71 deletions src/XML/src/FMI3/fmi3_xml_variable.c
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ fmi3_string_t* fmi3_xml_get_string_variable_start(fmi3_xml_string_variable_t* v)
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if(fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_string_variable_start_t* start = (fmi3_xml_string_variable_start_t*)(vv->type);
return (fmi3_string_t*)jm_vector_get_itemp(jm_voidp)(&start->stringStartValues, 0);
return (fmi3_string_t*)jm_vector_get_itemp(jm_voidp)(&start->values, 0);
}
return NULL;
}
Expand All @@ -785,7 +785,7 @@ fmi3_string_t* fmi3_xml_get_string_variable_start_array(fmi3_xml_string_variable
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if(fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_string_variable_start_t* start = (fmi3_xml_string_variable_start_t*)(vv->type);
return (fmi3_string_t*)jm_vector_get_itemp(jm_voidp)(&start->stringStartValues, 0);
return (fmi3_string_t*)jm_vector_get_itemp(jm_voidp)(&start->values, 0);
}
return NULL;
}
Expand Down Expand Up @@ -830,9 +830,9 @@ size_t fmi3_xml_get_binary_variable_start_size(fmi3_xml_binary_variable_t* v) {
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if (fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_binary_variable_start_t* start = (fmi3_xml_binary_variable_start_t*)(vv->type);
return start->nStart;
return (size_t)jm_vector_get_item(size_t)(&start->valuesSize, 0);
}
return 0;
return NULL;
}

size_t fmi3_xml_get_binary_variable_start_array_size(fmi3_xml_binary_variable_t* v) {
Expand All @@ -848,7 +848,7 @@ size_t* fmi3_xml_get_binary_variable_start_array_sizes(fmi3_xml_binary_variable_
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if (fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_binary_variable_start_t* start = (fmi3_xml_binary_variable_start_t*)(vv->type);
return (size_t*)jm_vector_get_itemp(size_t)(&start->binaryStartValuesSize, 0);
return (size_t*)jm_vector_get_itemp(size_t)(&start->valuesSize, 0);
}
return NULL;
}
Expand All @@ -857,7 +857,7 @@ fmi3_binary_t* fmi3_xml_get_binary_variable_start_array(fmi3_xml_binary_variable
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if (fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_binary_variable_start_t* start = (fmi3_xml_binary_variable_start_t*)(vv->type);
return (fmi3_binary_t*)jm_vector_get_itemp(jm_voidp)(&start->binaryStartValues, 0);
return (fmi3_binary_t*)jm_vector_get_itemp(jm_voidp)(&start->values, 0);
}
return NULL;
}
Expand All @@ -867,7 +867,7 @@ fmi3_binary_t fmi3_xml_get_binary_variable_start(fmi3_xml_binary_variable_t* v)
fmi3_xml_variable_t* vv = (fmi3_xml_variable_t*)v;
if (fmi3_xml_get_variable_has_start(vv)) {
fmi3_xml_binary_variable_start_t* start = (fmi3_xml_binary_variable_start_t*)(vv->type);
return start->start;
return (fmi3_binary_t)jm_vector_get_item(jm_voidp)(&start->values, 0);
}
return NULL;
}
Expand Down Expand Up @@ -1031,12 +1031,6 @@ static int fmi3_xml_hexstring_to_bytearray(fmi3_xml_parser_context_t* context, c
return 0;
}

static void free_string_start_values(jm_callbacks* callbacks, fmi3_xml_variable_t* var) {
fmi3_xml_string_variable_start_t* start = (fmi3_xml_string_variable_start_t*)(var->type);
jm_vector_foreach(jm_voidp)(&start->stringStartValues, callbacks->free);
jm_vector_free_data(jm_voidp)(&start->stringStartValues);
}

static bool is_string_variable_with_start(fmi3_xml_variable_t* var) {
return var->type->baseType == fmi3_base_type_str && (var->type->structKind == fmi3_xml_type_struct_enu_start);
}
Expand All @@ -1048,26 +1042,27 @@ static bool is_binary_variable_with_start(fmi3_xml_variable_t* var) {
void fmi3_xml_variable_free_internals(jm_callbacks* callbacks, fmi3_xml_variable_t* var) {
/* We need special handling of Strings and Binaries because
* of how start values are handled differently for those types.
* Note that below handles BOTH scalars and arrays.
*/
if (is_string_variable_with_start(var)) {
fmi3_xml_string_variable_start_t* start = (void*)(var->type);
jm_vector_foreach(jm_voidp)(&start->values, callbacks->free);
jm_vector_free_data(jm_voidp)(&start->values);
} else if (is_binary_variable_with_start(var)) {
fmi3_xml_binary_variable_start_t* start = (void*)(var->type);
jm_vector_foreach(jm_voidp)(&start->values, callbacks->free);
jm_vector_free_data(jm_voidp)(&start->values);
jm_vector_free_data(size_t)(&start->valuesSize);
}

if (fmi3_xml_variable_is_array(var)) {
if (is_string_variable_with_start(var)) {
free_string_start_values(callbacks, var);
} else if (is_binary_variable_with_start(var)) {
fmi3_xml_binary_variable_start_t* start = (fmi3_xml_binary_variable_start_t*)(var->type);
jm_vector_foreach(jm_voidp)(&start->binaryStartValues, callbacks->free);
jm_vector_free_data(jm_voidp)(&start->binaryStartValues);
jm_vector_free_data(size_t)(&start->binaryStartValuesSize);
} else {
if (!is_string_variable_with_start(var) && !is_binary_variable_with_start(var)) {
callbacks->free(fmi3_xml_get_variable_start_array(var));
}
callbacks->free(var->dimensionsArray);
var->dimensionsArray = NULL;
} else {
/* Scalar String variables can have a start value that we need to free.*/
if (is_string_variable_with_start(var)) {
free_string_start_values(callbacks, var);
}
}

if (var->clocks) {
jm_vector_free(fmi3_value_reference_t)(var->clocks);
var->clocks = NULL;
Expand Down Expand Up @@ -1766,18 +1761,19 @@ int fmi3_xml_handle_BinaryVariable(fmi3_xml_parser_context_t* context, const cha
startObj->nStart = nStart;

// number of bytes per element in context->currentStartVariableValues
jm_vector_init(jm_voidp)(&startObj->binaryStartValues, 0, context->callbacks);
jm_vector_init(size_t)(&startObj->binaryStartValuesSize, 0, context->callbacks);
jm_vector_init(jm_voidp)(&startObj->values, 0, context->callbacks);
jm_vector_init(size_t)(&startObj->valuesSize, 0, context->callbacks);
uint8_t* bytes;
for (int i = 0; i < nStart; i++) {
char* item = (char*)jm_vector_get_item(jm_voidp)(&context->currentStartVariableValues, i);
size_t sz = strlen(item)/((size_t)2);
bytes = context->callbacks->malloc(sz);
if (fmi3_xml_hexstring_to_bytearray(context, item, bytes)) {
fmi3_xml_parse_fatal(context, "Unable to convert hexstring to a byte array");
return -1;
}
jm_vector_push_back(jm_voidp)(&startObj->binaryStartValues, bytes);
jm_vector_push_back(size_t)(&startObj->binaryStartValuesSize, sz);
jm_vector_push_back(jm_voidp)(&startObj->values, bytes);
jm_vector_push_back(size_t)(&startObj->valuesSize, sz);

// We can now free the string now since we have converted it above
context->callbacks->free(item);
Expand Down Expand Up @@ -1848,9 +1844,9 @@ int fmi3_xml_handle_StringVariable(fmi3_xml_parser_context_t *context, const cha
fmi3_xml_parse_fatal(context, "Could not allocate memory");
return -1;
}
jm_vector_init(jm_voidp)(&startObj->stringStartValues, 0, context->callbacks);
// Passing ownership to stringStartValues
jm_vector_copy(jm_voidp)(&startObj->stringStartValues, &context->currentStartVariableValues);
jm_vector_init(jm_voidp)(&startObj->values, 0, context->callbacks);
// Passing ownership to values
jm_vector_copy(jm_voidp)(&startObj->values, &context->currentStartVariableValues);
variable->type = &startObj->super;
// Resize the vector to 0 since we are now done with the previous values.
jm_vector_resize(jm_voidp)(&context->currentStartVariableValues, 0);
Expand Down Expand Up @@ -1902,45 +1898,18 @@ int fmi3_xml_handle_BinaryVariableStart(fmi3_xml_parser_context_t* context, cons
fmi3_xml_type_definitions_t* td = &md->typeDefinitions;
fmi3_xml_variable_t* variable = jm_vector_get_last(jm_named_ptr)(&md->variablesByName).ptr;
if (!data) {
if (fmi3_xml_variable_is_array(variable)) {
/* For each <Start ...>, allocate memory, copy attribute to 'value' and push back to 'vec'. */
jm_vector(jm_voidp)* vec = &context->currentStartVariableValues;
const char* attr;
if (fmi3_xml_get_attr_str(context, fmi3_xml_elmID_BinaryVariableStart, fmi_attr_id_value, 0, &attr)) return -1;
int len = strlen(attr);
if (len == 0) {
fmi3_xml_parse_error(context, "Empty value attribute in Start element");
return -1;
}
char* attrAsStr = context->callbacks->malloc(len + 1);
strcpy(attrAsStr, attr);
jm_vector_push_back(jm_voidp)(vec, attrAsStr);
} else {
jm_vector(char)* bufStartStr = fmi3_xml_reserve_parse_buffer(context, 1, 100);
if (fmi3_xml_set_attr_string(context, fmi3_xml_elmID_BinaryVariableStart, fmi_attr_id_value, 0, bufStartStr)) {
return -1;
}

// Add a start object to the top of the variable's type list:
fmi3_xml_binary_variable_start_t* startObj;
size_t len = jm_vector_get_size(char)(bufStartStr);
if (len == 0) {
fmi3_xml_parse_error(context, "Empty value attribute in Start element");
return -1;
}
size_t arrSize = len / 2;
size_t totSize = sizeof(fmi3_xml_binary_variable_start_t) + arrSize;
startObj = (fmi3_xml_binary_variable_start_t*)fmi3_xml_alloc_variable_type_start(td, variable->type, totSize);
if (!startObj) {
fmi3_xml_parse_fatal(context, "Could not allocate memory");
return -1;
}
startObj->nStart = arrSize;
if (fmi3_xml_hexstring_to_bytearray(context, jm_vector_get_itemp(char)(bufStartStr, 0), startObj->start)) {
return -1;
}
variable->type = &startObj->super;
/* For each <Start ...>, allocate memory, copy attribute to 'value' and push back to 'vec'. */
jm_vector(jm_voidp)* vec = &context->currentStartVariableValues;
const char* attr;
if (fmi3_xml_get_attr_str(context, fmi3_xml_elmID_BinaryVariableStart, fmi_attr_id_value, 0, &attr)) return -1;
int len = strlen(attr);
if (len == 0) {
fmi3_xml_parse_error(context, "Empty value attribute in Start element");
return -1;
}
char* attrAsStr = context->callbacks->malloc(len + 1);
strcpy(attrAsStr, attr);
jm_vector_push_back(jm_voidp)(vec, attrAsStr);
}
return 0;
}
Expand Down