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

Review Summary: No summary


----------------------------------------
File: dialin/hd/hemodialysis_device.py
Revision Comment by Behrouz NematiPour on 07 April 2022, 14:21 
https://devapps.diality.us/cru/DIALIN-DEN-12358-1#c12771

  I don't see any reference/use to the created object.
  Why do we need to create all the objects while we don't even
  use them?
  This approach creates a lot of dependencies and also makes
  Dialin a very heavy API to load.

  Reply by Michael Garthwaite on 08 June 2022, 11:17
  > This is a QoL feature for users. Otherwise they would have to
  > create the CAN interface themselves and assign it themselves.
  > V&V has had issues like this before where they would create
  > VV_submodule_class and it ends up interfering with their data
  > collection.

    Reply by Behrouz NematiPour on 09 June 2022, 08:58
    > I not I understand the answer and how that related to my
    > question.
    > I think what I am trying to say is:
    >  - Why do we create an object that we have no use for it,
    > yet.
    >  - Why create an object for every class before even being
    > used.
    >  - Or Why it is not being created when it is needed and
    > where it is going to be used.
    > 
    > And could you please help understand what is "QoL feature"?

      Reply by Michael Garthwaite on 28 June 2022, 08:57
      > QoL feature = Quality of Life feature.
      > 
      > It is a feature for users (V&V and sys) that allows them
      > to exercise the API and generate tests faster. Breaking
      > this current design would cause some major refactoring
      > for them as they expect the HD and DG classes to have the
      > sub modules loaded.
      > 
      > We can look into the technical performance in
      > http://dvm-linux02:8080/browse/DIAL-172

        Reply by Behrouz NematiPour on 28 June 2022, 15:51
        > RESOLVED for now.

----------------------------------------
File: dialin/hd/alarms.py
Revision Comment by Behrouz NematiPour on 07 April 2022, 14:10 
https://devapps.diality.us/cru/DIALIN-DEN-12358-1#c12769

  It is big data to be reserved and filled with just default
  values, There should be a better way.

  Reply by Sean Nash on 11 May 2022, 08:46
  > I'm open to suggestions.  It's been this way forever and has
  > been working fine, so I'm also good leaving this as is.

    Reply by Behrouz NematiPour on 09 June 2022, 08:52
    > We should improve the Dialin coding someday!
    > RESOLVED

Revision Comment by Behrouz NematiPour on 07 April 2022, 13:57 
https://devapps.diality.us/cru/DIALIN-DEN-12358-1#c12768

  Please use conversions.py

  Reply by Sean Nash on 11 May 2022, 08:47
  > Mike, can you please look at updating these (and perhaps many
  > others throughout Dialin)?

    Reply by Michael Garthwaite on 12 July 2022, 09:51
    > In ticket http://dvm-linux02:8080/browse/DIAL-171

      Reply by Behrouz NematiPour on 12 July 2022, 12:04
      > Please take care of it as soon as you found time.
      > It's quite a bit of time since we talked about and It's a
      > month since I created a case for that.
      > 
      > RESOLVED for now.

Revision Comment by Behrouz NematiPour on 07 April 2022, 14:13 
https://devapps.diality.us/cru/DIALIN-DEN-12358-1#c12770

  Unnecessary/confusing use of ternary-if.
  please change to:
  {code}
  self.alarm_clr_top_only[alarm_id[0]] = alarm_cto[0] == 1
  {code}

  Reply by Sean Nash on 11 May 2022, 08:49
  > I'm ok with this either way.  Ternary should not be viewed as
  > confusing, but agree it is unnecessary.

    Reply by Behrouz NematiPour on 09 June 2022, 08:53
    > what I meant is the following statement is a binary why it
    > has been compared with True/False once, and put in trinary
    > once more just to compare a binary statement with binary.
    > {code}alarm_cto[0] == 1{code}
    > it is double the unnecessary work and confusing.

      Reply by Sean Nash on 14 June 2022, 09:30
      > alarm_cto[0] is an integer - not a boolean.
      > alarm_clr_top_only[] is an array of booleans.  
      > If Python boolean "True" is equivalent to integer "1",
      > then I suppose we can assign directly.
      > I think the current way this assignment is coded is
      > making no assumptions and explicitly converting integer
      > to boolean which is ok with me.

        Reply by Sean Nash on 14 June 2022, 09:49
        > Agree with Behrouz's initial proposed change above.

          Reply by Michael Garthwaite on 12 July 2022, 09:58
          > Fixed. Thanks!

          Reply by Dara Navaei on 19 October 2023, 13:02
          > RESOLVED

----------------------------------------
File: dialin/common/hd_defs.py
Revision Comment by Behrouz NematiPour on 07 April 2022, 13:30 
https://devapps.diality.us/cru/DIALIN-DEN-12358-1#c12767

  [~snash], [~dnavaei], 
  Please create a case for UI when something like the HD/DG
  changes in the common repository which definitely impacts UI
  behavior, and sometimes stops UI build.
  It should be done when the actual code is changed in the common
  repo, not the dialin.
  I am going to update UI now but the comment is for later.

  Reply by Sean Nash on 11 May 2022, 08:45
  > By "case", you mean a Jira bug?  Sure, we can do that.

    Reply by Behrouz NematiPour on 09 June 2022, 08:50
    > Yes, [~snash], a Jira task, I meant.
    > RESOLVED.


---

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

Title: DIALIN-DEN-12358_SW Dev Sprint 67 MG
Statement of Objectives:


State: Closed

Summary: 

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