This is a list of all comments for DIALIN-BUG-S63-1.

Review Summary: No summary


----------------------------------------
File: dialin/ui/hd_simulator.py
Revision Comment by Michael Garthwaite on 02 February 2022, 10:03 
https://devapps.diality.us/cru/DIALIN-BUG-S63-1#c11854

  Totally in favor of the workaround.

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

Revision Comment by Michael Garthwaite on 26 January 2022, 14:56 
https://devapps.diality.us/cru/DIALIN-BUG-S63-1#c11782

  Mode 4 for the HD is Treatment parameters. Every time this is
  called the UI will flicker to the Patient ID screen then back
  to Home. Is that what we want?

  Reply by Behrouz NematiPour on 01 February 2022, 16:44
  > That is correct and I have something in mind to improve the
  > code.
  > 
  > But I believe this is more related to Dialin bugs:
  > 1 - First one which we talked about that Dialin creates all
  > the objects even it is not being used in the __init__.py
  > files which cause different use-cases of the API to be not
  > compatible together and can't be used together.
  > 2 - the other issue is that the Dialin as an API should by
  > nature be a passive code and just have the capabilities to be
  > extended those observers have to be removed from Dialin.
  > 
  > For now, I'll put a workaround for this, and later we should
  > discuss the better implementation of the Dialin API.

  Reply by Behrouz NematiPour on 01 February 2022, 19:23
  > Fixed.

    Reply by Michael Garthwaite on 02 February 2022, 10:01
    > RESOLVED.

----------------------------------------
File: dialin/utils/conversions.py
Revision Comment by Dara Navaei on 20 January 2022, 12:51 
https://devapps.diality.us/cru/DIALIN-BUG-S63-1#c11769

  For this kind of logging, I think we should use the logger
  class.

  Reply by Behrouz NematiPour on 01 February 2022, 15:16
  > I don't think that is a good idea to get a hug class like
  > logging to be involved in here.
  > It would be better to use the logging in the class which
  > using these very basic functions.

    Reply by Dara Navaei on 01 February 2022, 20:53
    > RESOLVED.

Revision Comment by Dara Navaei on 20 January 2022, 12:53 
https://devapps.diality.us/cru/DIALIN-BUG-S63-1#c11770

  I don't think we should have a default for the packing in this
  case. I think the packing type should be provided, like I need
  'i' I will provide 'i' and if someone else wants 'I' then it
  should be provided. Although the number of bytes of b and B are
  the same, but it should be up to the developer to provide which
  of they want.  Also, for safety we should add the '<' to make
  sure the little endianness is definitely enforced. For instance
  it is '<i" and not just 'i'.

  Reply by Behrouz NematiPour on 01 February 2022, 15:20
  > Separate functions have been created to keep the string
  > representations clearer.
  > It is not safe to use something like "b" or "B" since anyone
  > can even but something like "hello" which probably doesn't
  > mean anything and is an error.
  > to overcome this flow the different functions implemented
  > here have to be used.
  > 
  > Also since the intention of the function here is clear the
  > length has been calculated regarding the type conversion in
  > each function. 
  > which helps have a lot of clearer code.
  > Please refer to the usage in:
  > d9b45e5b52db2540a79cbc8795d82dda4fd3984b on dialin develop
  > branch.
  > 
  > the code has changed from:
  > {code}
  >         major = struct.unpack('B', bytearray(
  > 
  > message['message'][self.START_POS_MAJOR:self.END_POS_MAJOR]))
  >         minor = struct.unpack('B', bytearray(
  > 
  > message['message'][self.START_POS_MINOR:self.END_POS_MINOR]))
  >         micro = struct.unpack('B', bytearray(
  > 
  > message['message'][self.START_POS_MICRO:self.END_POS_MICRO]))
  >         build = struct.unpack('H', bytearray(
  > 
  > message['message'][self.START_POS_BUILD:self.END_POS_BUILD]))
  >         compatibility = struct.unpack('H', bytearray(
  > 
  > message['message'][self.START_POS_COMPAT:self.END_POS_COMPAT]))
  > {code}
  > which was so error-prone to the following code:
  > {code}
  >         major, index = bytearray_to_byte(payload, index,
  > False)
  >         minor, index = bytearray_to_byte(payload, index,
  > False)
  >         micro, index = bytearray_to_byte(payload, index,
  > False)
  >         build, index = bytearray_to_short(payload, index,
  > False)
  >         compt, index = bytearray_to_integer(payload, index,
  > False)
  > {code}

    Reply by Dara Navaei on 01 February 2022, 20:53
    > RESOLVED.

----------------------------------------
File: dialin/ui/unittests.py
Revision Comment by Michael Garthwaite on 02 February 2022, 10:17 
https://devapps.diality.us/cru/DIALIN-BUG-S63-1#c11855

  Overall the use of this file is a bit funny. If this is ran
  within a Squish test, maybe we should rename/move the file (Ex:
  system_env_BAT.py) to be more coherent as it currently implies
  that it is unit testing something in this folder when it is
  testing the ENV and CAN.

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


---

ID: DIALIN-BUG-S63-1 https://devapps.diality.us/cru/DIALIN-BUG-S63-1

Title: DIALIN-BUG-S63
Statement of Objectives:


State: Closed

Summary: 

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