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

Review Summary: No summary


----------------------------------------
File: dialin/common/msg_ids.py
Revision Comment by Sean Nash on 08 December 2022, 14:19 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15174

  May not be needed, but can we just add 0xB8 too for
  completeness?

  Reply by Dara Navaei on 08 December 2022, 14:25
  > Done.

    Reply by Sean Nash on 08 December 2022, 14:28
    > RESOLVED.

----------------------------------------
File: dialin/hd/battery.py
Revision Comment by Sean Nash on 08 December 2022, 10:39 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15147

  I believe Dong Nguyen created this module (in 2022).

  Reply by Dara Navaei on 08 December 2022, 10:57
  > The authors and dates will be addressed automatically once
  > the copyright script is run in staging build.

    Reply by Sean Nash on 08 December 2022, 11:16
    > RESOLVED.

Revision Comment by Sean Nash on 08 December 2022, 10:39 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15146

  Should this be in the class?

  Reply by Dara Navaei on 08 December 2022, 10:59
  > Done.

    Reply by Sean Nash on 08 December 2022, 11:20
    > RESOLVED.

Revision Comment by Sean Nash on 08 December 2022, 10:35 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15142

  What is this doing here?  Should be above sync functions below
  with list of published vars.

  Reply by Dara Navaei on 08 December 2022, 11:03
  > That is right. Done.

    Reply by Sean Nash on 08 December 2022, 11:21
    > RESOLVED.

Revision Comment by Sean Nash on 08 December 2022, 10:34 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15141

  Functions need headers.

  Reply by Dara Navaei on 08 December 2022, 11:04
  > Done.

    Reply by Sean Nash on 08 December 2022, 11:22
    > first get function still missing header.

      Reply by Dara Navaei on 08 December 2022, 11:29
      > Please look at the green lines at the bottom of these
      > comments

        Reply by Sean Nash on 08 December 2022, 11:31
        > RESOLVED.

Revision Comment by Sean Nash on 08 December 2022, 10:38 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15145

  Remove extra blank line.

  Reply by Dara Navaei on 08 December 2022, 11:00
  > Done.

    Reply by Sean Nash on 08 December 2022, 11:20
    > RESOLVED.

Revision Comment by Michael Garthwaite on 08 December 2022, 10:45 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15149

  Needs @publish with message variables.

  Reply by Dara Navaei on 08 December 2022, 10:50
  > Done.

    Reply by Michael Garthwaite on 08 December 2022, 11:07
    > RESOLVED IN CODE WALKTHROUGH.

Revision Comment by Sean Nash on 08 December 2022, 10:37 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15143

  Remove dead code.

  Reply by Dara Navaei on 08 December 2022, 11:02
  > Done.

    Reply by Sean Nash on 08 December 2022, 11:21
    > RESOLVED.

Revision Comment by Michael Garthwaite on 08 December 2022, 10:45 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15148

  Needs @publish with message variables.

  Reply by Dara Navaei on 08 December 2022, 10:57
  > Done

    Reply by Michael Garthwaite on 08 December 2022, 11:07
    > RESOLVED IN CODE WALKTHROUGH.

Revision Comment by Michael Garthwaite on 08 December 2022, 10:47 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15150

  Does this work? struct.unpack does not have an HH option.

  Reply by Sean Nash on 08 December 2022, 11:16
  > Must have worked.  I think just one H is all that's needed
  > for unsigned short.  Not sure why type "int" is being parsed
  > as unsigned short though.

    Reply by Michael Garthwaite on 08 December 2022, 11:26
    > RESOLVED IN CODE WALKTHROUGH.

      Reply by Sean Nash on 08 December 2022, 11:32
      > Looking at unpack functionality a little more, I believe
      > HH will parse a 32-bit byte array as 2 unsigned shorts.
      > I think it's kind of working because byte array is little
      > endian and we're only looking at the first value (index
      > 0) parsed.  
      > Firmware is sending everything as U32.  So we should be
      > using "I" for unsigned int.

Revision Comment by Sean Nash on 08 December 2022, 10:37 
https://devapps.diality.us/cru/DIALIN-DEN-14001-1#c15144

  Remove dead code.

  Reply by Dara Navaei on 08 December 2022, 11:00
  > Done.

    Reply by Sean Nash on 08 December 2022, 11:20
    > RESOLVED.


---

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

Title: DIALIN-DEN-14001_HD Battery Driver Update
Statement of Objectives:


State: Closed

Summary: 

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