Yes to remove the merged code from Logging2of3 I had to create a new review. I looked into removing the your merged code from the old review and Crucible doesn't really let you if you create the review from a branch. So, the duplication of files does appear to be a bug in Crucible. The process isn't perfect in either case, but I think I addressed and committed changes for all of your comments from the previous review. If I missed anything please let me know
Behrouz NematiPour FYI, since the beginning of Dialin API when PeterL took over its product ownership, we made the decision to use Sphinx for its documentation because it is more suited for Python-based code. And, although Dialin API interacts with UI APP and DG/HD FW, it is a complete separate package of its own, which will never end in the final product, hence its categorization as non-product software.
So this is a Dialin handler to command the pump to a new set point (flow rate). I suppose it could have taken an unsigned rate and a separate direction parameter. Combining direction with target rate as a signed value reduced message size from Dialin. The Dialin command is commented to note that a negative rate indicates reverse direction.
The _FileSaver class does not provide a filename that it emits when a file is saved. It only emits a boolean of whether the file was saved. Also, connecting it to tst_views.cpp would not help because how do I know which file to check when my slot is called? Also, the slot would have to know which test needs to receive a signal from _FileSaver so it can evaluate the result. If the test should be self contained as you mentioned in the other comment then does it make sense to connect it to _FileSaver?
Hey Peter, You're right I removed them to easier do the merge. Since the task DEN-4690: Doxygenization has been changed to story and there is a code review sub-task for that, let's address all the documentation in that stoy.
1-byte alignment does not appear to be necessary here. If you want to save a little space, 2-byte alignment would make sense. But since we're not serializing these records (e.g. for transmission over CAN), we don't need to pack these records at all.
I have some questions regarding these libraries. 1 - What tool/code is checking for these requirements? 2 - What if the requirement is not met? (not a correct version or lib not exists) 3 - If the lib does not exist is it going to be installed or thrown an error? 4 - Don't we need to validate all these libraries? 5 - What is our process of adding more libraries to the list? On the other way is it fine to have all these libs? are all of them being used?
