Number | Date | Reviewer | Action | Response |
---|---|---|---|---|
1 | 6/5/1900 | Rob Lampereur | Create a linkage in the code between the Design Document and the 8051 assembly code. To make the logic flow easier to map between the design document and and code itself. Create labels within the code, which we reference within the document. |
Yes, this would be nice to do. A significant effort that will take some time. |
2 | 6/5/1900 | Rob Lampereur | Describe the Rational behind the Main Loop order, and that it is arbitrary. Basically, add into your comments that these are event driven activities - so the order is arbitray. | Comment added to MAINROUTINE comment block |
3 | 6/5/1900 | Art Rankin | Change comment in command reception task to state that the ICD command rate must be adhereed to, otherwise the condition of reading before writing could exist. If ICD rate met, then this condition should NOT exist. | Comment added to CHECKGP comment block |
4 | 6/5/1900 | Rob Lampereur | The CheckRXD Flag has been moved to the background activity, only because Daniel wanted to support FASTER comanding than has been documented that we need to support. Put into your comments why this has been moved into the background routine. |
Comment added to BACKGROUND comment block |
5 | 6/5/1900 | Chris Dorato | For every subroutine in the code, there should be a summary block describing what the subroutine is and what it does, what the inputs and outputs are, and what the revision history is - starting from this point in time onward. | Comment blocks added to every subroutine, some macro definitions, and mainloop. To properly fill-in each comment block will take some time, but I will revise comment blocks as development continues... |
6 | 6/5/1900 | Chris Dorato | The comment used elsewhere should be replaced with an explation of why it is done - explain why the PUSH and POP registers are done in the CHECKRXDFLAG subroutine. | 'used elsewhere' removed from comments, replaced with seemly more meaningful comments. |
7 | 6/5/1900 | Will Clement | Add comments to sections of code that do a check on the project, and make sure it is clear what is the FLIGHT configuration. Also, if code is ONLY for test - make sure it says so. Also, on line 4132 of EXECUTIV.LST, correct comment error. |
This probably had a specific reference, but not clear now. Fixed comment error in re: 'EX1' |
8 | 6/5/1900 | Chris Dorato | Document, at a minimum, in the five files, what are the key names/concepts from previous programs that are used in the comments. e.g., FUSE, IDS, Galex, 1553, DPU, etc... These concepts should be described in a banner message in the front of each file. |
The list has been started in the commands.inc file, which is read by all source files. |
9 | 6/5/1900 | Michelle Troeltzsch | Put comments in table definitions within the Patchable constants file. | A basic comment added to TABLE definitions in PATCHCST.a51 file. |
10 | 6/5/1900 | Allison Elliot | Change comment at 3792 t(of EXECUTIVE.LST) to note that sending only changed values of HSK pertains only to Galex. COS will transmit the entire packet every time. | Comment added to routine SENDHKP. |
11 | 6/5/1900 | Will Clement | Make SPARE_CODE label parameters be the difference of two variables, which the assembler computes... rather than having a hard-coded value. See floppy and hard-copy of code from Will C. |
Sorry, can't do with A51 v5.02 I called Archimedes about it, and they tell me I already have the latest version. The Franklin code example produces an error with the Archimedes A51 assembler. The fact is I'm missing my manuals and I ordered replacements from Archimedes, so I may learn how to do this trick later. |
12 | 6/5/1900 | Art Rankin | For the section of FSW that reads and populates the PH data buffers, look at the BOOT/OPERATE flag - and if in BOOT, do NOT read the PH data and do NOT overwrite the buffer area. | |
13 | 6/5/1900 | Michelle Troeltzsch | Any comments and more importantly, actually 8051 code, refererring to the 1553 interface used on FUSE should be updated for the COS communications protocol. | References to 1553 have been removed from the a51 files, but not the comments in common.inc |
14 | 6/5/1900 | Ken Brownsberger | If a properly formatted command is sent, but it does not exist in the OPCODE list, then report a diagnostic and an HST error. (Daniel is recommending Diagnostic 11 - command opcode not implemented.) | Separate COS and GALEX command tables have been implemented to support this AI. |
15 | 6/5/1900 | Daniel Blackman | SPARE commands currently in the command jump table should be replaced with room for future implementation of TBD commands. For now, they should be basically function as a NOOP - but, also report an illegal command diagnostic. (See previous AI). | |
16 | 6/5/1900 | Art Rankin | Confirm that a HSK packet is returned as a result of the JMP command. i.e., if we jump from BOOT to OPERATE, we get a HSK packet - without having to send another follow-up command, like a NOOP. | Test plan should include such a test. The expectation that a command might result in a jump to the REENTRY point is one of the reasons that CHECKGP is called first in the loop, see items 2,3 |
17 | 6/5/1900 | Rob Lampereur | Add functionality to make the Test Commands only usable if a TEST ENABLE command (a new command) has been sent. The default is that Test Commands are DISABLED. Also, when dropping to BOOT, Test Commands should be DISABLED. | OK, but not done yet. |
18 | 6/5/1900 | Daniel Blackman | Update comment in UPDATE_CTRS in EXECUTIV.LST. The Comment, 'Timer??? I think...' is not descriptive enough. | Comments rewritten. |
19 | 6/5/1900 | Michelle Troeltzsch | When reading the counters and the PHD, add comments that you are zero'ing the memory after you read them. |
Comments added. |
20 | 6/5/1900 | Rob Lampereur | Set the Highwater mark to just below MAX in the PROM, but copy it over to the patchable constants area, and use this value for your CHECK_STACK code. This way, you can patch it to a low value to verify that the DIAGNOSTIC 7 function is working properly. Also, add a TLM item for the stack max, so that this can be bundled with DIAG 7 for use with an HST error. |
Great idea. I wish I had the time to implement and test this, but basic functionality gets first priority . I can probably get it working before the test proedure for it can get written. The second idea is also good, but not yet implemented. |
21 | 6/5/1900 | Will Clement | Revise the format of the Jump Table. See code on floppy disk and hard-copy from Will C. | Jump Table format is completely redesigned, but not quite the way Will had specified. JMPs were competely eliminated to create a lookup table of command subroutine entry-points. The CHECK_COMMAND routine in commands.a51 was changed to fetch the subroutine address and jump there. The PROJECT bit is used to select the command table, allowing separate command tables for GALEX and COS. The RET instruction at the end of every command subroutine returns control to the MAINLOOP caller. |
22 | 6/6/1900 | Michelle Troeltzsch | Add more text to the interrupt comments (starting at line 2252 of INITIALZ.LST). Explain what the interrupt does. If need additional space, put in on the line above. |
Comments added. |
23 | 6/6/1900 | Michelle Troeltzsch | Scrub all bug and debug comments throughout the entire code package. If a problem that your tracking with these kinds of comments still exists at the time the code is ready to go to Level 2 CM, then the problem needs to be tracked offically and paper-work generated. Also, dont leave dangling comments about bugs and possible fixes. If the bug was fixed, update the comment and say it was fixed. |
Debug comments or lines removed throughout. Word 'debug' is used in some comments that do not actually indicate debug code. Other debug code is simply commented out, but preserved to demonstrate how some debug function could be performed. |
24 | 6/6/1900 | Art Rankin | Generate a lien list of bugs or problems that exist in the code when it is brought to Level 2. Maintain that list and update it appropriately so that it can be reviewed at the time the code is brought to Level 3. (Level 3 = Delivery to BATC.) | OK, but not done yet. |
25 | 6/6/1900 | Rob Lampereur | Line 3380 of INITIALZ.LST. Add comments that this section of code ONLY executes for development builds. When it is compiled for flight, INIT_TO will have a value such that the LOAD_RAM_CODE is NEVER executed. | This is actually achieved during the link/locate phase. The code build process creates four flavors of FSW: boot-mode image, boot-to-operate-mode image, lower upload image, and upper upload image. By selecting the boot-mode image to create PROMs, this code will perform as required. The boot-to-operate image meets GALEX requirements, and both lower and upper upload images are suitable for GALEX or COS. There is a bit in housekeeping that indicates we are in Boot or Operate mode. |
26 | 6/6/1900 | Ken Brownsberger | Remove the functionality at line 3389 of INITIALZ.LST. Do NOT perform the NOTSAFE. We will ALWAYS shut off HV, Door, etc... | The XPSEC assembly control shows the old code and the new code. I want to document how I would have designed to code to be tolerant of some kinds of errors, because I believe it to be a better, safer design for general use. However, I will acquiesce to the review board and leave XSPEC turned off for flight code and qual testing. XSPEC will be turned on for use in the lab during detector development. Warning: leaving these features out of the boot PROM removes any possibility of allowing the detector to continue transmitting science data after a reset, even a spurious reset caused by SEU. |
27 | 6/6/1900 | Art Rankin | Move the LOAD_RAM_CODE - for development only - to the last part of the Initialize sequence. Even though we will never do this in operations, the early hardware development would benefit by execution in a fashion that is more like how we will operate the detector. *** IGNORE *** After further discussion, it was realized that this is NOT possible, given that the code image Daniel was referring to REQUIRES operation out of the RAM area. |
Ignored as specified. |
28 | 6/6/1900 | Ken Brownsberger | Line 4392 of COMMANDS.LST Add comment in front banner that CDC is really referring to TDCs. We are not flying CDCs in the FUV Detector. Also, add comment here that you are initializing the three-wire interface for the Digitizers. |
Comments added as requested. |
29 | 6/6/1900 | Rob Lampereur | Lines 4378 of COMMANDS.LST Fix the code if the lock bits are corrected in hardware. Do not unlock the Protected Control Register, unless you need it. If hardware is not corrected, document WHY you are doing what you are doing |
Added comments to explain why LOCK bit brackets access to either controls register. |
30 | 6/6/1900 | Daniel Blackman | Line 4385, COMMANDS.LST Strike comment about I wonder if we should do this. |
Comments struck. The sentiment remains. |
31 | 6/6/1900 | Rob Lampereur | Lines 2584 and on, in INITIALZ.LST Add comments in FINDRESETTYPE, which indicate what kind of reset took place - and why. |
Comments added |
32 | 6/6/1900 | Michelle Troeltzsch | Change text of Record this dreadful event, to something along the lines of issue a DCE diagnostic, and - if appropirate - Report an HST error. | Replaced the word 'dreadful' with 'diagnostic'. This is a macro that does a general purpose function. Appropriate comments are near the macro call describe the nature of the diagnostic.. |
33 | 6/6/1900 | Allison Elliot | The Watchdog should be disabled after ALL resets, not just a POR. | Acquiescence. See Item 26. |
34 | 6/6/1900 | Ken Brownsberger | Assigned to: Ken B. Update DCE-Reset diagram to include the 255 POR check. |
|
35 | 6/6/1900 | Ken Brownsberger | Since the PROM OFF funtionality has been removed, remove the IVTINDIR at line 3037 of INITIALZ.LST Also, the IVTPATCH and IVTINDIR on line 4626 of EXECUTIV.LST. Are these needed? Consensus is that we shouldnt be running BOOT CODE ISRs out of RAM. Execute the BOOT code out of PROM. |
The programmer does not agree, but may acquesce later with the same understanding as Item 26. Not done yet, until the programmer is sure the review board fully understands that ISRs cannot be changed in flight, if this action is implemented as is. |
36 | 6/6/1900 | Ken Brownsberger | Flight CODE should not decide what flight hardware were running on because of hardware talkbacks, and then set behavior for future operations because of this determiniation. The SETPROJECT, at line 4184, functionality should NOT test the RXD1 line and determine whether or not the DCE is COS or Galex. A suggestion was made to use conditional compiles to generate code sets that are unique to the different PROJECTS, and different lab hardware environments. This is to remove the possibility of executing the wrong code set for COS on the flight hardware. (Either during test - or worse - on orbit.) |
Implemented as per item 26. |
37 | 6/6/1900 | Rob Lampereur | Line 3406 of INITIALZ.LST Change comment from Hang the processor, to Simulate a hung processor by not stroking the watchdog for 10 seconds. |
Comments added. |
38 | 6/6/1900 | Ken Brownsberger | CHECKBREADBOARD, at line 2965 of INITIALZ.LST Do NOT check hardware, and then determine flight software functionality based on this check... COS behavior should be burned into PROM, and we should be guarranteed COS behavior. (See previous AI.) |
Removed CHECKBREADBOARD from code |
39 | 6/6/1900 | Rob Lampereur | Line 3204, (CLRSOMEBITS) in INITIALZ.LST. Document which bits youre clearing and why theyre being cleared. |
Comments added |
40 | 6/6/1900 | Rob Lampereur | The Initialization of the CRP task, and setting the HV Status Bits needs to be added to the Initalization sequence. They are currently baselined for the design, but have not been included into the code. (e.g., See Fig 4.2-1 of SDD) | |
41 | 6/6/1900 | Ken Brownsberger | Assigned to Ken B: Change 12 seconds to 10 seconds in Figure 4.2-1 of SDD. Remove background calculations in 4.2.2.4.1 of SDD. Remove the Start CRC calculations in 4.2.2.4.2, in the minimal inizationzation sequence. (Change to Setup or Initialize for CRC calculations.) |
|
42 | 6/6/1900 | Ken Brownsberger | Possibly remove INIT_TEST_BKGRND, at line 3035 of INITIALZ.LST - as were not going to do any Background activity in Reset Initialization. It is called in REENTRY anyway. | Should be no problem, but item 26 applies. |
43 | 6/6/1900 | Michelle Troeltzsch | Remove CLR EA, at line 4557 in EXECUTIV.LST. As we just finished calling it at 4545. | CLR EA removed. |
44 | 6/6/1900 | Michelle Troeltzsch | Try to scrub any and all comments in code that are no longer valid - especially those comments that are invalid because the code the comment was referring has been removed. | At least some comments of this sort have been removed from code. |
45 | 6/6/1900 | Michelle Troeltzsch | Remove comment on 4625 that says debug. The HKPFULL is now the default design. | Comment rewritten. |
46 | 6/6/1900 | Ken Brownsberger | Assigned to Ken B.: Is the protected control register wired incorrectly for the DCE? If so, when is this going to be fixed? Controls 1 should have been locked. Controls 2 currently has the lock. Consult with John Andrews, Barry Welsh, and electrical engineers working at UCB. Daniel currently implements an undesirable work-around because of this problem. Make certain that he only does this if it the hardware is NOT going to be fixed. If the hardware does not get fixed - and the wrong controls register is stuck with the lock... then consider unlocking it initially, and keeping it unlocked indefinitely. |
We aren't going to fix this in hardware. |
47 | 6/6/1900 | Ken Brownsberger | Assigned to Ken B.: Is there a problem in the Actel for the PROM enable? Does UCB have a problem with the PROM that has not been documented, and that Daniel has created code to fix? |
|
48 | 6/6/1900 | Michelle Troeltzsch | The word debug in a comment means that this line of code needs to be revisited. No words debug should exist in the comments when the code is delivered to BATC and GSFC. (See previous AIs on this topic.) | Debug comments or lines removed throughout. Word 'debug' is used in some comments that do not actually indicate debug code. Other debug code is simply commented out, but preserved to demonstrate how some debug function could be performed. |
49 | 6/6/1900 | Allison Elliot | Add a banner comment to the command traffic ISRs on 2307 of INTERNAL.LST. (See previous AI on this topic) | Subroutine banners added, to be filled-in during development |
50 | 6/6/1900 | Ken Brownsberger | Assigned to Ken B. Get the COMMON.INC file and scrub it with the existing DCE HW/SW Interface Document. Provide any inputs/updates to Rich B. |
|
51 | 6/6/1900 | Art Rankin | Add a counter to the Command Traffic ISR, so that we can keep track of how many Command words are rejected by the CHECKRXD routine. (Line 2417 of INTERNAL.LST) | Good idea, not yet done. Requires housekeeping assignment, updates to documentation. |
52 | 6/6/1900 | Ken Brownsberger | Assigned to Ken B.: Work with Daniel to fix Logic Flow of Timer Tick ISR in SDD. Current flow diagram does not match 8051 code. |
|
53 | 6/6/1900 | Allison Elliot | Comments starting on 4500 of COMMAND.LST are outdated and should be updated. | Comments removed and updated. |
54 | 6/6/1900 | Allison Elliot | There needs to be logic at line 4489 of COMMAND.LST, so that the SETB F0 is not set when the CRC parameter is not set to zero. |
Code is written but left out until a suitable test can be written to show that it works. The upload code would better be left unmolested, but the programmer will acquiesce to the review board's desires (and DM-05 command description). |