This is a list of all comments for UI-DEN-1434-1. Review Summary: No summary ---------------------------------------- File: sources/canbus/messageinterpreter.cpp Revision Comment by Behrouz NematiPour on 11 March 2020, 12:39 https://devapps.diality.us/cru/UI-DEN-1434-1#c1426 Please put range check. Reply by Behrouz NematiPour on 11 March 2020, 12:49 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:39 https://devapps.diality.us/cru/UI-DEN-1434-1#c1427 Please put range check. Reply by Behrouz NematiPour on 11 March 2020, 12:49 > RESOLVED Revision Comment by pmontazemi on 10 March 2020, 09:58 https://devapps.diality.us/cru/UI-DEN-1434-1#c1381 Space between cases, everywhere. Reply by Behrouz NematiPour on 10 March 2020, 10:25 > Some of the comments about the spacing are in our c coding > standards and not in our C++ coding standard. > From the beginning we discussed that these may/should be > different. Reply by Behrouz NematiPour on 10 March 2020, 12:42 > applied regardless. Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:51 https://devapps.diality.us/cru/UI-DEN-1434-1#c1371 Add space between two if conditions. Reply by Behrouz NematiPour on 10 March 2020, 12:48 > applied . Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:51 https://devapps.diality.us/cru/UI-DEN-1434-1#c1372 Add space before if condition. Reply by Behrouz NematiPour on 10 March 2020, 12:48 > applied . Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:51 https://devapps.diality.us/cru/UI-DEN-1434-1#c1373 Add space between two if conditions. Reply by Behrouz NematiPour on 10 March 2020, 12:47 > applied . Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:51 https://devapps.diality.us/cru/UI-DEN-1434-1#c1374 Add space between two if conditions. Reply by Behrouz NematiPour on 10 March 2020, 12:47 > applied. Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:52 https://devapps.diality.us/cru/UI-DEN-1434-1#c1375 Same here. Reply by Behrouz NematiPour on 10 March 2020, 12:47 > applied . Reply by pmontazemi on 10 March 2020, 14:53 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:52 https://devapps.diality.us/cru/UI-DEN-1434-1#c1376 Add space. Reply by Behrouz NematiPour on 10 March 2020, 12:46 > applied. Reply by pmontazemi on 10 March 2020, 14:54 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:52 https://devapps.diality.us/cru/UI-DEN-1434-1#c1377 Add space. Reply by Behrouz NematiPour on 10 March 2020, 12:44 > applied . Reply by pmontazemi on 10 March 2020, 14:54 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:52 https://devapps.diality.us/cru/UI-DEN-1434-1#c1378 Add space. Reply by Behrouz NematiPour on 10 March 2020, 10:27 > Peman, > Some of the comments about the spacing are in our c coding > standards and not in our C++ coding standard. > From the beginning we discussed that these may/should be > different. Reply by Behrouz NematiPour on 10 March 2020, 12:43 > applied regardless. Reply by pmontazemi on 10 March 2020, 14:54 > RESOLVED. ---------------------------------------- File: sources/canbus/messagedispatcher.cpp Revision Comment by pmontazemi on 10 March 2020, 09:10 https://devapps.diality.us/cru/UI-DEN-1434-1#c1367 Remove unused #defines. Reply by Behrouz NematiPour on 10 March 2020, 12:49 > This is not unused. > This is for debugging if un-commented. Reply by pmontazemi on 10 March 2020, 14:52 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:11 https://devapps.diality.us/cru/UI-DEN-1434-1#c1368 Remove extra line. Reply by Behrouz NematiPour on 10 March 2020, 12:48 > applied . Reply by pmontazemi on 10 March 2020, 14:55 > RESOLVED. ---------------------------------------- File: sources/canbus/caninterface.cpp Revision Comment by pmontazemi on 10 March 2020, 09:07 https://devapps.diality.us/cru/UI-DEN-1434-1#c1365 Extra line needed at EOF. Reply by Behrouz NematiPour on 10 March 2020, 12:00 > It has EEL, Extra Empty Lines are not necessary shown up in > crucible. Reply by pmontazemi on 10 March 2020, 14:52 > RESOLVED. ---------------------------------------- File: sources/canbus/caninterface.h Revision Comment by pmontazemi on 10 March 2020, 09:07 https://devapps.diality.us/cru/UI-DEN-1434-1#c1366 Extra line needed at EOF. Reply by Behrouz NematiPour on 10 March 2020, 12:50 > There is one. > Not visible here. Reply by pmontazemi on 10 March 2020, 14:55 > RESOLVED. ---------------------------------------- File: sources/applicationcontroller.cpp Revision Comment by Behrouz NematiPour on 11 March 2020, 12:44 https://devapps.diality.us/cru/UI-DEN-1434-1#c1432 it is in a loop with int i as loop variable. inner loop should not use the same even if the outer loop variable in not use in inner. Reply by Behrouz NematiPour on 11 March 2020, 12:49 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:44 https://devapps.diality.us/cru/UI-DEN-1434-1#c1433 it is in a loop with int i as loop variable. inner loop should not use the same even if the outer loop variable in not use in inner. Reply by Behrouz NematiPour on 11 March 2020, 12:50 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:42 https://devapps.diality.us/cru/UI-DEN-1434-1#c1429 Should be 0, 1, 2, 3 Index 4 is out of bound. Reply by Behrouz NematiPour on 11 March 2020, 12:50 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:43 https://devapps.diality.us/cru/UI-DEN-1434-1#c1430 it is in a loop with int i as loop variable. inner loop should not use the same even if the outer loop variable in not use in inner. Reply by Behrouz NematiPour on 11 March 2020, 12:50 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:40 https://devapps.diality.us/cru/UI-DEN-1434-1#c1428 Should be 0, 1, 2, 3 Index 4 is out of bound. Reply by Behrouz NematiPour on 11 March 2020, 12:50 > RESOLVED Revision Comment by Behrouz NematiPour on 11 March 2020, 12:43 https://devapps.diality.us/cru/UI-DEN-1434-1#c1431 it is in a loop with int i as loop variable. inner loop should not use the same even if the outer loop variable in not use in inner. Reply by Behrouz NematiPour on 11 March 2020, 12:51 > RESOLVED ---------------------------------------- File: scripts/run.sh Revision Comment by pmontazemi on 10 March 2020, 09:05 https://devapps.diality.us/cru/UI-DEN-1434-1#c1363 finally with two ls. Reply by Behrouz NematiPour on 10 March 2020, 12:51 > Thanks. > Applied. Reply by pmontazemi on 10 March 2020, 14:52 > RESOLVED. Revision Comment by pmontazemi on 10 March 2020, 09:06 https://devapps.diality.us/cru/UI-DEN-1434-1#c1364 Remove extra line at EOF. Reply by Behrouz NematiPour on 10 March 2020, 12:50 > applied . Reply by pmontazemi on 10 March 2020, 14:56 > RESOLVED. ---------------------------------------- File: sources/canbus/frameinterface.h Revision Comment by plucia on 10 March 2020, 09:42 https://devapps.diality.us/cru/UI-DEN-1434-1#c1369 Should this be hardcoded to 4000? Would it ever need to change to 10,000? Reply by Behrouz NematiPour on 10 March 2020, 11:57 > This value came out of no where. > It may change later and I don't currently know what value > preferred. Reply by plucia on 10 March 2020, 12:03 > Could it be the same as txqueuelen? That's how I > interpreted it Reply by Behrouz NematiPour on 10 March 2020, 14:04 > Although they are doing almost the same thing but one is > for driver buffer and this one is for application buffer > and are not necessarily the same. Reply by plucia on 10 March 2020, 15:46 > RESOLVED ---------------------------------------- File: sources/utility/types.h Revision Comment by pmontazemi on 10 March 2020, 09:55 https://devapps.diality.us/cru/UI-DEN-1434-1#c1379 Space. Reply by Behrouz NematiPour on 10 March 2020, 10:13 > I don't see where needs space. Reply by pmontazemi on 10 March 2020, 10:21 > Before the first if. Reply by Behrouz NematiPour on 10 March 2020, 12:43 > applied. Reply by pmontazemi on 10 March 2020, 14:55 > RESOLVED. ---------------------------------------- File: sources/view/vpoweroff.cpp Revision Comment by pmontazemi on 10 March 2020, 09:55 https://devapps.diality.us/cru/UI-DEN-1434-1#c1380 Add extra line at EOF. Reply by Behrouz NematiPour on 10 March 2020, 10:11 > It has EEL, Extra Empty Lines are not necessary shown up in > crucible. Reply by pmontazemi on 10 March 2020, 14:55 > RESOLVED. --- ID: UI-DEN-1434-1 https://devapps.diality.us/cru/UI-DEN-1434-1 Title: UI-DEN-1434_InTreatment Statement of Objectives: State: Closed Summary: Author: Behrouz NematiPour Moderator: Behrouz NematiPour Reviewers: (1 active, 2 completed*) plucia (*) pmontazemi (*) Dara Navaei