Update on write_mef_ts_data_and_indices function

[MaxvandenBoom]

Hey Dan,

I was working with the write_mef_ts_data_and_indices function and saw that there were improvements possible (which are in my code as well). Mainly, the uh pointer variable is used to point to the universal-headers of multiple files (generic fps, data-file fps and index-file fps), making the code difficult to follow and prone to errors. I simplified and corrected some empty operations:

• Simplified the changes to the time-series index fps. Skipped assignment: uh = ts_idx_fps->universal_header; That reassignment get's overwritten 6 lines later (to ts_data_fps) and we can do direct referencing. Which results in less "reassignments" of the uh pointer variable.

• Assigning values to the start_time and end_time of the universal header of the generic fps (uh pointer variable at the beginning of the function):

  uh->start_time = metadata_fps->universal_header->start_time;
  uh->end_time = metadata_fps->universal_header->end_time;

  does not make sense since nothing relevant happens to the generic fps and uh after. The uh pointer variable is "repointed" to ts_data_fps->universal_header shortly after and gen_fps is never used again. The start_time and end_time values are actually transferred from the metadata file at the line:

  FILE_PROCESSING_STRUCT *ts_data_fps = allocate_file_processing_struct(UNIVERSAL_HEADER_BYTES + RED_MAX_COMPRESSED_BYTES(samples_per_block, 1), TIME_SERIES_DATA_FILE_TYPE_CODE, NULL, metadata_fps, UNIVERSAL_HEADER_BYTES);

• I think part of the code that makes use of the universal header of the generic fps (gen_fps) is copied over from the write_mef_ts_metadata function, in which gen_fps and it's universal header are actually used. As a result of this copying some code does not do anything or can be simplified:

  • The extractions of the segment number (uh->segment_number = extract_segment_number(&name[0]);), channel name (MEF_strncpy(uh->channel_name, name, MEF_BASE_FILE_NAME_BYTES);) and session name (MEF_strncpy(uh->session_name, name, MEF_BASE_FILE_NAME_BYTES);) are stored in the universal header of the generic mef fps (gen_fps) but are never used anywhere. Which makes sense because these fields are copied later from metadata file fps/universal header. These extractions can just be removed.
  • As you probably already felt and described in a comment in the code (you wrote: // NOTE: gen_fps is unecessart here if the metadata file with the universal header already exists, or is it?). The generic MEF fps/uh ('gen_fps') is not very relevant for the function, except to provide a universal header: uh = gen_fps->universal_header; to generate a password on: pwd = process_password_data(NULL, level_1_password, level_2_password, uh); Indeed all is based on the metadata file universal header and this is just copied over from the 'write_mef_ts_metadata' function. Therefore we can simplify the password part to not use 'uh' as an extra step and just write: pwd = process_password_data(NULL, password_l1, password_l2, gen_fps->universal_header);

• Now after all of the above steps, the 'uh' variable is only used for the time-series fps, at: uh = ts_data_fps->universal_header; Since it will only refer to that, I renamed that variable to ts_data_uh to make the code more readable

• Last, I updated the comments in the function to describe a more of what is going on

I tested the code so it should be good.

Best, Max

[cimbi]

agreed. I am working on PyMED now and I see a lot of improvements for PyMEF too. I came across the ones you mentioned a few days ago. Also it does not make sense to separate the functions for metadata, data and indices writing. Happy to merge this