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

Review Summary: No summary


----------------------------------------
File: dialin/ui/hd_simulator.py
Revision Comment by plucia on 24 May 2021, 08:13 
https://devapps.diality.us/cru/DIALIN-DEN-7820-1#c10045

  The message ID should not be passed to the command. Each dialin
  function should correspond to a single message ID.
  The channel ID should not be passed to the command. This should
  be defined in the dialin command. The HD and DG simulators are
  to be separated into different classes. If you pull staging
  you'll see what I mean.

  Reply by Behrouz NematiPour on 24 May 2021, 08:52
  > This message is a very dynamic message for rapid testing.
  > A screen in the simulator gives the user the ability to set
  > the source, type of data, and length of the message, and
  > nothing is known in this dialin command.
  > So this is the required structure of the function.
  > I can't imagine how else we could have a function to support
  > that dynamic screen.

    Reply by plucia on 24 May 2021, 09:30
    > I find it very hard to believe it wouldn't be possible to
    > support that screen and not pass the message id and channel
    > ID to this function. 
    > One function should not be supporting multiple message IDs.
    > You just need more functions to support that screen.

      Reply by Behrouz NematiPour on 24 May 2021, 23:24
      > It has been updated regarding our conversation.

        Reply by plucia on 25 May 2021, 10:51
        > The fromHD flag should be removed since there is a
        > separate dg simulator
        > Typing information still needs to be added

          Reply by Behrouz NematiPour on 25 May 2021, 13:15
          > It has been addressed in another code review for
          > another branch (TxLog , Disinfection).

          Reply by plucia on 28 May 2021, 17:22
          > RESOLVED

Revision Comment by plucia on 25 May 2021, 10:50 
https://devapps.diality.us/cru/DIALIN-DEN-7820-1#c10109

  The fromHD flag should be removed since there is a separate dg
  simulator

  Reply by Behrouz NematiPour on 25 May 2021, 13:15
  > It has been addressed in another code review for another
  > branch (TxLog , Disinfection).

    Reply by plucia on 28 May 2021, 17:22
    > RESOLVED

Revision Comment by plucia on 25 May 2021, 10:46 
https://devapps.diality.us/cru/DIALIN-DEN-7820-1#c10108

  Typing information needs to be added for all new "cmd" methods
  to be consistent with the rest of dialin. The older commands
  can be updated at a later time, but new ones should follow this
  standard.

  Reply by Behrouz NematiPour on 25 May 2021, 13:16
  > It has been addressed in another code review for another
  > branch ( TxLog, Disinfection ).
  > If an API is missing that shall be addressed in a separate
  > task.

    Reply by plucia on 28 May 2021, 17:22
    > RESOLVED


---

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

Title: DIALIN-DEN-7820_UI DEV Post Treatment
Statement of Objectives:


State: Closed

Summary: 

Author: Behrouz NematiPour
Reviewers: (0 active, 2 completed*)
     plucia   (*)
     pmontazemi   (*)