-
Notifications
You must be signed in to change notification settings - Fork 9
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
Trend channel fixes #405
base: master
Are you sure you want to change the base?
Trend channel fixes #405
Conversation
…ls for information
…nel description rather than characteristic to grab characteristic and type information
return channel => | ||
!channel.Trend && | ||
channel.MeasurementType.Name == "Digital" && | ||
measurementCharacteristics.Contains(channel.MeasurementCharacteristic.Name); | ||
channel.MeasurementCharacteristic.Name == "Instantaneous"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filter will match ALL digitals, not just triggers. It will also match triggers that don't have a trend channel classification, such as the 3I0
triggers I see in my example file from TVA. This is a problem, because the logic will issue a warning to the console for every one of those channels that cannot be resolved, and it will misclassify them as RMS Voltage to create a bunch of trend channels that should not exist.
If you insist on doing away with the measurement characteristics that we used for the trigger digitals, then we need to tweak the logic to use your regex matching in the filter. Also, it might be prudent to run the change by Tony since he will no longer have any control over which channels this logic will target when adding trend channels.
@@ -989,7 +980,7 @@ private void AddTriggerChannel(Channel powChannel, MeterDataSet meterDataSet) | |||
MeasurementTypeID = powChannel.MeasurementTypeID, | |||
MeasurementCharacteristicID = powChannel.MeasurementCharacteristicID, | |||
PhaseID = powChannel.PhaseID, | |||
Name = powChannel.Name, | |||
Name = powChannel.Name + "Trigger", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This creates a channel name like T1Trigger
, right? Maybe T1Trend
would make more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to add as a suggestion.
Name = powChannel.Name + "Trigger", | |
Name = powChannel.Name + "Trend", |
@@ -1008,49 +999,47 @@ private void AddTriggerChannel(Channel powChannel, MeterDataSet meterDataSet) | |||
TableOperations<Channel> channelTable = new TableOperations<Channel>(connection); | |||
TableOperations<Series> seriesTable = new TableOperations<Series>(connection); | |||
|
|||
trendChannel.MeasurementType = measurementTypeTable.QueryRecordWhere("ID = {0}", trendChannel.MeasurementTypeID); | |||
trendChannel.MeasurementCharacteristic = measurementCharacteristicTable.QueryRecordWhere("ID = {0}", trendChannel.MeasurementCharacteristicID); | |||
string measCharacteristic = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're changing all this code anyway, I'd prefer if we spelled it out.
string measCharacteristic = ""; | |
string measurementCharacteristic = ""; |
measCharacteristic = "Instantaneous"; | ||
measurementType = "Analog"; | ||
} | ||
else if (Regex.IsMatch(trendChannel.Description, " I.+\\s*$$", RegexOptions.IgnoreCase)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On further inspection, I wonder if Tony was using Trigger - I
for 3I0
. If so, this regex pattern wouldn't match it.
APP frequency, rms, and trigger trend COMTRADE files were not being processed into HIDS or properly forming XDA Trend Channels.
This PR is currently on the demo system and data is being seen by influxDB.
https://gridprotectionalliance.atlassian.net/browse/XDA-44