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

Add support for multi-source messages and signal filtering based on source information #209

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Monicadepucchio
Copy link

The purpose of this Pull Request is to enhance our understanding of the source message for different signals in our application.
In our application, the source message is closely tied to the device used for signal acquisition. In certain cases, the device used for acquisition aligns with the source of the message.
For instance, consider a signal named "speed". In addition to the signal itself, it is important to know the source message, which corresponds to the acquisition device.
All the changes made have been carried out in parallel to the normal code flow.
The code changes are as follows:

  1. Mdf.py: We have added a new input called "source_list" (default is None). It is a string list with all the sources’ messages. Is not crucial that either it has the same signal list length or the source name has a unique occurrence
  2. Mdfinfo4: We have introduced a boolean variable called "multi-source". This variable is created at runtime. By default, it is set to False, but if there is a source list in the Mdf class, it becomes True. In cases where multi-source is True, the signal name will include the source information only if it is a time channel. Otherwise, only the signal name is considered. This information is appended when the signal is saved. Additionally, for MDF 4.x versions, it is now possible to filter signals based on their source information.
  3. MDF4reader: If the source list is present in the mdf class, the information block from the file is organized based on the different source messages. This allows us to match any signal with its corresponding source message (i.e., the device it comes from). We have excluded the occurrence of the source message in the source list, focusing solely on the time channel. The variable "own_source" describes the source message. If the evaluated channel is not a time channel and the source list is not None, the source message information is concatenated to the signal name string.

…ne). Mdfinfo4: We have introduced a boolean variable called "multi-source". This variable is created at runtime.MDF4reader: If the source list is present in the mdf class, the information block from the file is organized based on the different source messages.
@@ -2305,7 +2373,10 @@ def _unique_channel_name(self, fid, name, dg, cg, cn):
source_name = dg
else:
source_name = dg
name = u'{0}_{1}_{2}'.format(name, dg, source_name)
if multi_sources and name != 't':
Copy link
Owner

Choose a reason for hiding this comment

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

This is very specific to me. I suppose "t" is time. Can very different system by system, not really portable

Copy link

Choose a reason for hiding this comment

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

I've made an adjustment concerning the master channel(time in my acquisitions), aiming for a unique name for that channel. Specifically, I replaced it with "nfo['masters'][info['DG'][dataGroup]['dg_cg_first']]['name']". I would value your insights and thoughts on this modification. If you have any suggestions or considerations regarding this change, please feel free to share.

find_own_source = False
skip_this_group = False
# if there is no signal in the channel group
if len(channel_set & info['ChannelNamesByDG'][dataGroup]) < 2 and 't' in list(
Copy link
Owner

Choose a reason for hiding this comment

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

It could be t but also time, Time or another, not so robust

Copy link

Choose a reason for hiding this comment

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

I've made an adjustment concerning the master channel(time in my acquisitions), aiming for a unique name for that channel. Specifically, I replaced it with "self['masters'][self['CG'][dg-1][cg]['pointer']]['name']". I would value your insights and thoughts on this modification. If you have any suggestions or considerations regarding this change, please feel free to share.

filter_channel_names=filter_channel_names, minimal=minimal)
else:
info = self.info
# If the source list is present, the information block from file are organised
Copy link
Owner

Choose a reason for hiding this comment

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

I do not understand this change, you can pass over multi_sources parameter being None or some proper content

Copy link

Choose a reason for hiding this comment

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

Upon reviewing the code again, I realized that I wrote it hastily. I have revised this section as follows:

if self.info is None:
if source_list is None:
multi_sources = False
else:
# Extract the information block from the file and arrange it based on the provided source list.
# This organization enables us to associate each signal with its respective source message,
# indicating the device of origin.

    multi_sources = True

info = Info4(self.fileName, None,
             filter_channel_names=filter_channel_names, minimal=minimal,
             multi_sources=multi_sources)

else:
info = self.info

In this revision, I added a has_source_list flag to clearly indicate whether a source list is present. If you have any further suggestions or if there's anything else you'd like me to address, please feel free to let me know. I appreciate your feedback.

@@ -91,7 +91,7 @@ class MdfSkeleton(dict):

def __init__(self, file_name=None, channel_list=None, convert_after_read=True,
filter_channel_names=False, no_data_loading=False,
compression=False, convert_tables=False, metadata=2):
compression=False, convert_tables=False, metadata=2, source_list=None):
Copy link
Owner

Choose a reason for hiding this comment

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

What type is actually expected, List or Set of strings ? From the modifications you propose, I think Set is more suitable as there is no need for ordering. It will allow more performing search than List.

Copy link

Choose a reason for hiding this comment

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

I appreciate your insights, and I've taken note of the comment in the code. I've already incorporated the suggested changes. If there's anything more you'd like me to address or if you have further recommendations, please feel free to let me know. I value your input and am open to any additional guidance you may have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants