CMUAbstract / tab

TAB reference implementations, examples, and documentation
Other
0 stars 0 forks source link

Add clock initialization function to COM example #3

Closed mablem8 closed 1 year ago

mablem8 commented 1 year ago

In com.c, add the function implementation for init_clock.

Test the com_monolithic.c program after implementing init_clock.

AbhishekCMU commented 1 year ago

Commit hash: 000c582

issue 2: 000c58211ac4742c30115833f1375e1a1dd57131

link: https://github.com/CMUAbstract/tab/issues/3#issue-1574705524

Added init_clock implementation in "\tab\c-examples\com-monolithic\support\com.c" Added led blink implementation in "\tab\c-examples\com-monolithic\com_monolithic.c"

mablem8 commented 1 year ago

Commit 000c58211ac4742c30115833f1375e1a1dd57131 addresses this issue with line 44 and line 45.

However, 000c58211ac4742c30115833f1375e1a1dd57131 introduces a regression by removing functions rx_usart1, reply, and tx_usart1.

Further, 000c58211ac4742c30115833f1375e1a1dd57131 introduces extraneous code in com_monolithic.c from line 27 through line 39. While this code may be useful for testing, it cannot be present in a TAB example program. In com.c, line 47 is extraneous as well.

Add the clock initialization function implementation to a commit in the sprint-1_abhi branch. Omit the superfluous code and the regression. Then post a link to the new commit as a comment in this issue thread.

AbhishekCMU commented 1 year ago

Removed the extraneous code for functions rx_usart1, reply, and tx_usart1 from com_monolithic.c, . In com.c and . In com.c

Removed the extraneous code from com_monolithic.c keeping only clock_init() (clock initilization) Removed the extraneous code from com.c, clock_init() function

Changes are committed to issue 3 branch. Merged issue 3 branch to sprint-1_abhi

mablem8 commented 1 year ago

When committing an issue fix, avoid merging branches on your own. As a general rule, to submit a fix to an issue:

  1. Branch from the active sprint branch (currently, sprint-2)
  2. Make a commit addressing the issue in the branch you just created
  3. Post a link to that commit in the corresponding issue thread for evaluation. In GitHub, simply pasting the commit hash into the comment field will automatically convert it into a link.

I will then check the linked commit and provide feedback. For the comment posted immediately above, its links are still to the old commit. When you have pushed your new commit, be sure to use updated links to that new commit when making comments.

AbhishekCMU commented 1 year ago

Commit Hash for Issue 3 fixes: 08d10d6

Removed the extraneous code for functions init_uart, rx_usart1, reply, and tx_usart1 from com_monolithic.c, com.c and com.h

Removed the extraneous code from com_monolithic.c keeping only clock_init() (clock initilization) Removed the extraneous code from com.c, clock_init() function

Changes are committed to sprint-2_abhi branch.

mablem8 commented 1 year ago

Looking much better! Do not remove init_uart, rx_usart1, reply, and tx_usart1 because they are essential to the example program and will eventually need to be added back if they are removed. It's ok if the empty function implementations generate compiler warnings for now.

For the implementation of init_clock, be sure to adhere to the style guide. Specifically, indentation should be two spaces and not \t characters.

For the next commit, I suggest starting a new branch from sprint-2 again because otherwise you'd have to work through the process to squash commits and might run into challenges if the restored init_uart, rx_usart1, reply, and tx_usart1 functions are even slightly different from the originals.

AbhishekCMU commented 1 year ago

Commit Hash: e65d681

Added clock initialization to init_clock function in com.c.

Changes are committed to sprint-2_abhi2_issue3 branch.

mablem8 commented 1 year ago

The solution compiles, and no testing is required to resolve this issue. Resolution accepted and issue closed.