FC 1240

>> FC 1240 has been removed from the Pixel PLC code.
>> We will use time servers for the synchronization of PLC time and PVSS time in the future.

FC 1400

Number 1

>> The reason why we decided to use TIME_OF_DAY was that it uses less memory than DATE_AND_TIME
>> (4 bytes rather than 8 bytes) and that monitoring of variables by means of VAT Variable Tables
>> is easier for variables of type TIME_OF_DAY than for variables of type DATE_AND_TIME
>> (TIME_OF_DAY can be monitored directly as time; 
>>  for DATE_AND_TIME, years, months, days, hours, minutes and seconds all have to be monitored individually).
>> 
>> We agree, however, that these disadvantages are only minor
>> and that it would be better to represent all times by variables of type DATE_AND_TIME.
>> The code has been changed accordingly.

Number 2

(see previous annotation)

Number 3-6

>> The code has been refactored (following your suggestions).
>> FC 1401 is actually called from FC 1400 only.
>> In order to reduce the number of functions/functions calls,
>> the computation of the "durationStateNotOk" value is performed directly in FC 1400 now.

Number 7

>> The "beenActive" flag is set to true when the Alarm condition is fullfilled.
>> In order for the "beenActive" flag to be reset, the Alarm condition must not be fullfilled (anymore) 
>> **and** the operator need to have acknowledged the Alarm via the PVSS graphical user interface.

FC 1401

>> The code in this function has been moved to FC 1400
>> and FC 1401 has been removed from the Pixel PLC code.

FC 402

Number 1.1

>> The byte to int conversion cannot fail.
>> BYTE_TO_INT just takes the 8 bits which represent the byte, adds 8 bits worth of zeros 
>> and interprets the resulting bit pattern as a 16 bit integer.
>> We believe that the conversion does not take much time (if any at all).

Number 1.2

(see previous annotation)

Number 1.3

>> Done. Thank you for the suggestion.

Number 2.1-2.2

(see previous annotation)

Number 2.3

>> "numDigInput_CfgPar_set" counts how many Digital Input channels 
>> with **valid** Configuration Parameters have been processed.

Number 2.4

>> The clearing can be done at Configuration time of course.
>> Personally, we prefer the current implementation rather than introducing a "coupling" between FC 102 and FC 402.
>> (FC 402 is supposed to be the **one and only** function for writing into DB 402.)

Number 2.5

>> Yes, the Acknowledgement persists for one OB 1 cycle only.
>> This is the intended behaviour.
>>
>> We agree that the command flags stored in DB 202 do not actually need to be reset in FC 402 
>> as they are reinitialized at the beginning of the next OB 1 cycle 
>> by values uploaded from PVSS (stored in DB 602) anyway.
>> Thank you for pointing this out.
>> (remark: similar resettings of commands were implemented in FC 1xx, FC 301, FC 308 and FC 4xx;
>> the code for resetting of command flags have been removed from all these functions.)

Number 2.6

>> We agree that the Data-Block header information is neccessary only for the Configuration Parameter 
>> Data-Blocks DB 6xx and is not neccessary for the Data-Blocks DB 4xx, which store process value information.
>> The header has been removed from the definition of DB 402 now.
>> (remark: the header has actually been removed from all DB 4xx.)

FC 404

(see previous annotation)

FC 410

Number 1.1

(see previous annotation)

Number 1.2

>> The limits for "rawTemperature" values are (implicitely) defined in HW-Config,
>> by configuring the sensor connected to the RTD channel as being of type "Pt1000"
>> (Platimum RTD temperature sensor with a nominal resistance of 1k Ohm).
>> The Limits are -12000 < rawTemperature < +13000
>> (values have been taken from Siemens S7 documentation;
>> the code is correct, but the comment above the code was wrong; 
>> the comment has been corrected now).
>> In case the "rawTemperature" is outside that range, 
>> the calibration parameters might be wrong.
>> 
>> The code has been simplified now by replacing the IF...THEN...ELSE construct
>> by an assignment, as you suggested.
>>
>> In the PLC code version which has been subject of the review,
>> the "Sensor_outsideMeasRange" flag had no effect on the interlock decision
>> and was computed for display by PVSS only.
>> In the revised version of the CMS Pixel PLC code the sensor is considered to be in Alarm condition
>> in case the "Sensor_outsideMeasRange" flag is set.

Number 1.3

>> In some cases, the usage of local variables really is required
>> (the SCL code would not compile otherwise, due to some technical limitations the details of which we are actually not aware of).
>> In most cases, the local variables have simply been introduced in order to avoid repetition of the the rather time-consuming
>> calculation od Data-Block addresses for expressions of the type "DBxxx.a[index].b".
>> We believe that the reduction in memory consumption that we would gain in avoiding the usage of local variables
>> is not worth the effort of changing the current code at this time. 

Number 1.4

(annotation)

Number 2.1

>> A channel is not OK unless it is in range **and fullfills all the other conditions, too !!**
>>
>> Personally, we find the current implementation of setting the OK flag to true first
>> and then checking individual conditions that might cause the OK flag to be set to false
>> easier to read compared to evaluating a single, complex expression.
>> (We propose to continue with the current implementation in this case.)

Number 2.2

(see previous annotation)

Number 2.3

>> A check if the "Sensor_outsideMeasRange" flag is set has been added now.
>> (cfg. response to FC 410, number 1.2)
>> Thank you for bringing this issue to our attention.

FC 411

Number 1.1

(see previous annotation)

Number 1.2

>> The limits for "rawHumidity" values are (implicitely) defined in HW-Config,
>> by configuring the analog input channel to measure currents in the range 4..20mA .
>> The Limits are -27648 < rawTemperature < +27648
>> (values have been taken from Siemens S7 documentation).
>>
>> The code has been simplified now by replacing the IF...THEN...ELSE construct
>> by an assignment, as you suggested.
>>
>> A check for "Sensor_outsideMeasRange" has been added;
>> the analog input channel is considered to be in Alarm condition
>> in case the "Sensor_outsideMeasRange" flag is set.
>> (cfg. response to FC 410, numbers 1.2 and 2.3)

Number 2.1

>> We are not sure we do understand your comment:
>> The reference temperature is needed in order to compute the dew-point.

Number 2.2

>> The code has been changed to use the OK flag instead of an "isValid" output parameter now.

Number 2.3

(see previous annotation)

Number 3.1

>> Yes, numHumidSens_processed = numHumidSens_CfgPar_set + numHumidSens_CfgPar_Err .
>> The number of channels processed is counted independently in the code
>> mainly to provide a cross-check.

Number 4.1

"The if-then-else structure of these functions is somewhat harder to follow than it needs to be. One way to improve this condition is to make channel processing functions and just have these functions drive over the list of channels."
>> The disadvantage of this change would be that it increases the total number of functions.
>> The current number of functions was critizized as being too large by Piero Giorgio Verdini.
>> (We propose to continue with the current implementation in this case.)
"It looks like the names are common amongst the reading functions, so there might be more opportunities to make common handling functions."
>> Some code for the channel processing really is specific to temperature sensors,
>> humidity sensors and digital input channels, respectively.
>> This makes it difficult to replace the code currently implemented in FC 402, FC 410 and FC 411 
>> by common handling functions.
>> (We propose to stay with the current implementation in this case.)