This is a list of all comments for DIALIN-DEN-5751-1. Review Summary: No summary General Comment by Behrouz NematiPour on 25 January 2021, 16:52 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7594 Also mentioned in the objectives: The only files that were involved in these changesets are: - dialin/ui/*hd_simulator.py* - dialin/common/*msg_defs.py* - dialin/squish/*denaliMessages.py* And all the other files are merged from master and other branches which were merged into master. ---------------------------------------- File: dialin/squish/denaliMessages.py Revision Comment by plucia on 29 January 2021, 09:49 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7644 These should be added to hd_simulator_alarms.py not denaliMessages.py. Reply by Behrouz NematiPour on 01 February 2021, 13:33 > UI_DVT related. Reply by plucia on 01 February 2021, 17:50 > will be addressed later, > RESOLVED ---------------------------------------- File: dialin/protocols/CAN.py Revision Comment by pmontazemi on 29 January 2021, 17:35 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7682 Replace with "Cannot" (no use of informal language in code comments) Reply by Behrouz NematiPour on 01 February 2021, 13:40 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:43 > RESOLVED. Revision Comment by pmontazemi on 29 January 2021, 17:36 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7683 Replace let's with let us. Reply by Behrouz NematiPour on 01 February 2021, 13:40 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:42 > RESOLVED. ---------------------------------------- File: tests/test_gen_requirements.py Revision Comment by pmontazemi on 29 January 2021, 17:39 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7686 Delete all commented out lines or add TODO add top of section. Reply by Behrouz NematiPour on 01 February 2021, 13:35 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:42 > RESOLVED. ---------------------------------------- File: tests/test_hd_simulator.py Revision Comment by pmontazemi on 29 January 2021, 17:39 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7687 Remove extra lines. Reply by Behrouz NematiPour on 01 February 2021, 13:35 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:42 > RESOLVED. ---------------------------------------- File: dialin/dg/conductivity_sensors.py Revision Comment by pmontazemi on 29 January 2021, 17:19 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7680 Why CD2 is commented out everywhere in this code? Reply by Behrouz NematiPour on 01 February 2021, 13:42 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:43 > RESOLVED. ---------------------------------------- File: dialin/hd/alarms.py Revision Comment by pmontazemi on 29 January 2021, 17:25 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7681 Should replace this hard-coded number with the exact number of alarms from len(dict). Reply by Behrouz NematiPour on 01 February 2021, 13:41 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:43 > RESOLVED. ---------------------------------------- File: tests/set_accels_cal.py Revision Comment by pmontazemi on 29 January 2021, 17:38 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7685 Delete all commented code lines below or add TODO next to each section. Reply by Behrouz NematiPour on 01 February 2021, 13:37 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:42 > RESOLVED. ---------------------------------------- File: tests/uf_test.py Revision Comment by pmontazemi on 29 January 2021, 17:40 https://devapps.diality.us/cru/DIALIN-DEN-5751-1#c7688 Remove commented out lines or add TODO on top of section. Reply by Behrouz NematiPour on 01 February 2021, 13:35 > Please refer to the general comment I put, > These codes are not maintained by UI. > {quote} > The only files that were involved in these changesets are: > > dialin/ui/hd_simulator.py > dialin/common/msg_defs.py > dialin/squish/denaliMessages.py > And all the other files are merged from master and other > branches which were merged into master. > {quote} Reply by pmontazemi on 01 February 2021, 13:42 > RESOLVED. --- ID: DIALIN-DEN-5751-1 https://devapps.diality.us/cru/DIALIN-DEN-5751-1 Title: DIALIN-DEN-5751_Pre Treatment Ultrafiltration Statement of Objectives: The only files that were involved in these changesets are: - dialin/ui/*hd_simulator.py* - dialin/common/*msg_defs.py* - dialin/squish/*denaliMessages.py* And all the other files are merged from master and other branches which were merged into master. - Added cmd_set_treatment_adjust_ultrafiltration_init_response - Fixing priming message ID - This was causing the tst_confirmprimingbegin not to receive the priming status message therefore couldn't reach 100% coverage. - moved the cr shell script into the tools folder. - added the message-id 80 for prTx UF adj rsp. - Added API for the message 63: Alarm Condition Cleared - Added AlarmFlags - Added Alarms Priority enums in the common folder. - Updated the CANBus test function test_can0 to be able to be used outside of the SquishQt (ex. Simulator) and moved the common portion in the utils.py State: Closed Summary: Author: Behrouz NematiPour Reviewers: (0 active, 2 completed*) plucia (*) pmontazemi (*)