This is a list of all comments for DIALIN-DEN-12121-1.

Review Summary: No summary


----------------------------------------
File: dialin/ui/hd_simulator.py
Revision Comment by Behrouz NematiPour on 04 March 2022, 00:31 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12359

  Please add the simulator functions for the newly added models,
  like the:
   - DG Conductivity Data
   - HD blood leak data
   - HD air bubbles data
   - Air Trap Data
   -  .....

  Reply by Michael Garthwaite on 09 March 2022, 11:37
  > Implemeted. DG Conductivity data is within dg_simulator.
  > Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:24
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:19 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12352

  Please update the docstring here and on top.

  Reply by Michael Garthwaite on 09 March 2022, 11:31
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:24
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:26 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12353

  Please update docstring here and above.

  Reply by Michael Garthwaite on 09 March 2022, 11:31
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:25
    > *Please remove the state in the @param state docstring.*
    > 
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:27 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12354

  Please update doctring here and above.

  Reply by Michael Garthwaite on 09 March 2022, 11:31
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:27
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:30 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12357

  the docstring says it is bool, which is correct.

  Reply by Michael Garthwaite on 09 March 2022, 11:36
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:27
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:29 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12356

  Please align the pipes ( | )
  fix #5 to #6

  Reply by Michael Garthwaite on 09 March 2022, 11:31
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:28
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:30 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12358

  update the docstring.

  Reply by Michael Garthwaite on 09 March 2022, 11:32
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:28
    > *Later please be consistent with types, (int) is used
    > everywhere*
    > 
    > RESOLVED
    > Thanks,

----------------------------------------
File: dialin/dg/hd_proxy.py
Revision Comment by Behrouz NematiPour on 04 March 2022, 00:37 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12363

  To be consistent with rest of the code and what has been
  decided use the standard docstring format as:
  @param (int): <description>
  @return (int): <description>

  Reply by Michael Garthwaite on 09 March 2022, 11:34
  > Updated doc string to format that methods follow. Note: none
  > follow 
  > @param (int): <description>
  > @return (int): <description>
  > 
  > They are:
  > @param: (int) <description>
  > @return: (int) <description>

    Reply by Behrouz NematiPour on 09 March 2022, 12:18
    > RESOLVED
    > Thanks,

----------------------------------------
File: dialin/common/hd_defs.py
Revision Comment by Dara Navaei on 08 March 2022, 20:34 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12428

  For consistency, HD_PRIME_PAUSE and HD_PRIME_COMPLETE should
  have STATE at the end of their names.

  Reply by Michael Garthwaite on 09 March 2022, 11:35
  > Fixed. Thanks!

    Reply by Dara Navaei on 09 March 2022, 12:33
    > RESOLVED.

----------------------------------------
File: dialin/common/msg_ids.py
Revision Comment by Dara Navaei on 08 March 2022, 20:37 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12429

  This should be 0xA04F.

  Reply by Michael Garthwaite on 09 March 2022, 11:35
  > Using 0xA008 as it is available instead

    Reply by Dara Navaei on 09 March 2022, 12:33
    > RESOLVED.

----------------------------------------
File: dialin/hd/post_treatment.py
Revision Comment by Behrouz NematiPour on 04 March 2022, 00:34 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12361

  unnecessary empty lines.

  Reply by Michael Garthwaite on 09 March 2022, 11:33
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:20
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:33 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12360

  please follow the standard:
  @retrurn: (int) <description>

  Reply by Michael Garthwaite on 09 March 2022, 11:33
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:19
    > RESOLVED
    > Thanks,

Revision Comment by Behrouz NematiPour on 04 March 2022, 00:35 
https://devapps.diality.us/cru/DIALIN-DEN-12121-1#c12362

  Please use the new conversion methods, from now on.
  I believe we decided to use them until you find time to update
  all the others.

  Reply by Michael Garthwaite on 09 March 2022, 11:33
  > Fixed. Thanks!

    Reply by Behrouz NematiPour on 09 March 2022, 12:20
    > RESOLVED
    > Thanks,


---

ID: DIALIN-DEN-12121-1 https://devapps.diality.us/cru/DIALIN-DEN-12121-1

Title: DIALIN-DEN-12121_SW Dev Sprint 65
Statement of Objectives:


State: Closed

Summary: 

Author: Michael Garthwaite
Moderator: Michael Garthwaite
Reviewers: (1 active, 2 completed*)
     Dara Navaei   (*)
     Behrouz NematiPour   (*)
     Sean Nash