This is a list of all comments for UI-DEN-704-1. Review Summary: No summary ---------------------------------------- File: scripts/run.sh Revision Comment by lbaloa on 05 December 2019, 13:35 https://devapps.diality.us/cru/UI-DEN-704-1#c491 The message should be more along the lines that CAN interface is running... Reply by Behrouz NematiPour on 19 December 2019, 09:01 > I meant : > "Can Interface has been setup" > > like that I have as : > "candump launched" Reply by lbaloa on 20 December 2019, 08:23 > RESOLVED ---------------------------------------- File: scripts/setupSD.sh Revision Comment by lbaloa on 05 December 2019, 13:37 https://devapps.diality.us/cru/UI-DEN-704-1#c493 Add commented line with the name of the script. Don't we need headers for these scripts? Reply by Dara Navaei on 19 October 2023, 10:57 > RESOLVED ---------------------------------------- File: sources/canbus/caninterface.cpp Revision Comment by lbaloa on 05 December 2019, 13:38 https://devapps.diality.us/cru/UI-DEN-704-1#c494 Device creation is too generic, make it more specific "Can interface cannot be initialize" Reply by Behrouz NematiPour on 05 December 2019, 13:47 > Can Interface has its own error information. > What I added here is an extra information. Reply by lbaloa on 20 December 2019, 08:22 > RESOLVED Revision Comment by lbaloa on 05 December 2019, 13:39 https://devapps.diality.us/cru/UI-DEN-704-1#c495 Same as before. Methods must be verbs. Reply by Behrouz NematiPour on 05 December 2019, 13:46 > I was going to use connect but the parent has one then it > will override it which is wrong. > So I changed it to connection. Reply by Behrouz NematiPour on 05 December 2019, 13:52 > Oh I changed it to initConnections() in new pushes as part > of DEN-1090 : DEN-1090-1 when I saw your comment. Reply by lbaloa on 20 December 2019, 08:22 > RESOLVED Revision Comment by lbaloa on 05 December 2019, 13:40 https://devapps.diality.us/cru/UI-DEN-704-1#c496 Line up onRead() slot with frameReceived()... e.g. onCANFrameReceived() Reply by Behrouz NematiPour on 05 December 2019, 13:49 > It has been done in new codes for DEN-1090 : DEN-1090-1. > But as a general comment its my preferred coding style for > readability and its not our rule or requirement. Reply by lbaloa on 20 December 2019, 08:21 > RESOLVED ---------------------------------------- File: sources/canbus/messagehandler.cpp Revision Comment by pmontazemi on 25 November 2019, 15:10 https://devapps.diality.us/cru/UI-DEN-704-1#c339 eChild? Reply by Behrouz NematiPour on 26 November 2019, 11:32 > it's eChlid_HD. > it has been defined in the header file as enum. > e : is for enum > Chl : is for Channel > id : is id > _HD : is HD device > So it's a message recieved from HD channel id whcih has been > defined as 0x020 in header file enum Reply by pmontazemi on 26 November 2019, 12:55 > RESOLVED. Revision Comment by pmontazemi on 25 November 2019, 15:10 https://devapps.diality.us/cru/UI-DEN-704-1#c338 isUnknown? Reply by Behrouz NematiPour on 26 November 2019, 11:36 > it has been defined in the header file as enum. > if the command or message Id which has been received is not > known by the Message Handler then it is 'isUnknow' Reply by Behrouz NematiPour on 26 November 2019, 11:47 > Sorry for the Typo ... > Fixed... Reply by pmontazemi on 26 November 2019, 12:55 > RESOLVED. ---------------------------------------- File: sources/canbus/messagehandler.h Revision Comment by pmontazemi on 25 November 2019, 15:10 https://devapps.diality.us/cru/UI-DEN-704-1#c341 eChild? Reply by Behrouz NematiPour on 26 November 2019, 11:37 > it's eChlid_HD. > it has been defined in the header file as enum. > e : is for enum > Chl : is for Channel > id : is id > _UI : is UI device > So it's a message sent out from UI channel id which has been > defined as 0x100 in header file enum Reply by pmontazemi on 03 December 2019, 10:41 > RESOLVED. Revision Comment by lbaloa on 05 December 2019, 13:50 https://devapps.diality.us/cru/UI-DEN-704-1#c500 Recommend using a more descriptive naming. 0x020 is an input channel to UI, i.e., HD2UI. Similar is 0x100, it is an output channel.From UI to HD. Reply by lbaloa on 19 December 2019, 08:47 > Behrouz 0x20 is a channel ID that is from UI to HD. This > identifier does not reflect that. Please reconsider > renaming. Reply by Behrouz NematiPour on 19 December 2019, 08:56 > For the two eChlid_UI and eChlid_HD which are for UI in UI > I prefer to keep them simple to use and read. Reply by Dara Navaei on 19 October 2023, 10:57 > RESOLVED Revision Comment by pmontazemi on 25 November 2019, 15:11 https://devapps.diality.us/cru/UI-DEN-704-1#c342 eChild? Reply by Behrouz NematiPour on 26 November 2019, 11:38 > t's eChlid_HD. > it has been defined in the header file as enum. > e : is for enum > Chl : is for Channel > id : is id > _UI : is UI device > So it's a message sent out from UI channel id which has been > defined as 0x100 in header file enum Reply by pmontazemi on 03 December 2019, 10:41 > RESOLVED. ---------------------------------------- File: sources/gui/guicontroller.cpp Revision Comment by lbaloa on 05 December 2019, 13:53 https://devapps.diality.us/cru/UI-DEN-704-1#c502 Same comment as before. Verb. Reply by Behrouz NematiPour on 19 December 2019, 08:54 > it has been addresses : > - http://dvm-linux02:8060/project/UI-DEN-1090 Reply by lbaloa on 20 December 2019, 08:20 > RESOLVED --- ID: UI-DEN-704-1 https://devapps.diality.us/cru/UI-DEN-704-1 Title: UI-DEN-704_HomeScreen Statement of Objectives: State: Closed Summary: Author: Behrouz NematiPour Moderator: Behrouz NematiPour Reviewers: (1 active, 2 completed*) pmontazemi (*) lbaloa (*) Dara Navaei