Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation file for Uart and example code. #204

Merged
merged 4 commits into from
Oct 28, 2018

Conversation

Jeremy-Chau
Copy link
Contributor

Implementation file for Uart and example code.

@Jeremy-Chau
Copy link
Contributor Author

Khalil,
I can't figure out how to properly implement the Pin Pairs lookup table. I guess I don't quite understand how to use a look up table of function calls. I thought I could use the table in the same manner as I did before creating the table but the build errors state otherwise. For reference, below is how I was able to setup the pins before creating the table.

    tx_pin_ = Pin(kTxChannel[mode_], kTxPortPin[mode_]);
    rx_pin_ = Pin(kRxChannel[mode_], kRxPortPin[mode_]);

@kammce
Copy link
Member

kammce commented Oct 15, 2018

Khalil,
I can't figure out how to properly implement the Pin Pairs lookup table. I guess I don't quite understand how to use a look up table of function calls. I thought I could use the table in the same manner as I did before creating the table but the build errors state otherwise. For reference, below is how I was able to setup the pins before creating the table.

    tx_pin_ = Pin(kTxChannel[mode_], kTxPortPin[mode_]);
    rx_pin_ = Pin(kRxChannel[mode_], kRxPortPin[mode_]);

With the way you are doing this, you can omit the tx_pin_ and the rx_pin_, even though this should work. Simply use a map of Uart Pins and assign their pointers to the tx_ and rx_ pointers.

- Moved color range from lowest red = 50 and highest necessary green to
  90.
- Precision has been moved to 1 decimal place. This should result in
  deltas less than .1% not be considered as coverage failures.
@codecov-io
Copy link

codecov-io commented Oct 25, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@96e59c3). Click here to learn what that means.
The diff coverage is 79.5%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #204   +/-   ##
========================================
  Coverage          ?   84.9%           
========================================
  Files             ?      43           
  Lines             ?    2025           
  Branches          ?       0           
========================================
  Hits              ?    1719           
  Misses            ?     306           
  Partials          ?       0
Impacted Files Coverage Δ
firmware/library/L1_Drivers/uart_test.cpp 100% <100%> (ø)
firmware/library/L1_Drivers/pin.hpp 100% <100%> (ø)
firmware/library/L1_Drivers/uart.hpp 69.3% <69.1%> (ø)
firmware/library/L1_Drivers/gpio.hpp 90% <85.8%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96e59c3...7313ce4. Read the comment docs.

@coveralls
Copy link

coveralls commented Oct 25, 2018

Coverage Status

Coverage decreased (-2.6%) to 80.913% when pulling 7313ce4 on Jeremy-Chau:UART into 96e59c3 on kammce:master.

@kammce
Copy link
Member

kammce commented Oct 25, 2018

Even though coveralls says you decreased coverage by 2.6%, I will still merge you code in because it drops the binary size considerable (~3kB I think).

@Jeremy-Chau
Copy link
Contributor Author

Were you able to properly verify that the code works on the board?

@kammce
Copy link
Member

kammce commented Oct 25, 2018

Haven't been able to get the driver working yet. Will test more tomorrow.

@kammce
Copy link
Member

kammce commented Oct 26, 2018

Figured out the issue. It mostly had to do with the fact that the UART0 pins were not correct. UART0 must be P0.2 and P0.3 since we need that for serial communication with host machine. Which means that the pin function bits were also off. After changing those and a few other things I got it to work.

See master's version of "Jeremy-Chau-UART" and make those changes to your repo. Ignore the changes to Hyperload though. I will make those changes after this is merged in.

@Jeremy-Chau
Copy link
Contributor Author

I believe I got all the changes in this updated commit. Please let me know if I missed any.

Copy link
Member

@kammce kammce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not go make the pull requests idential. Do not delete uart0.hpp and uart2.hpp. Just make the changes you need to, to uart.hpp.

uint8_t receiver = 0;
uint64_t timeout_time = Milliseconds() + timeout;
uint64_t current_time = Milliseconds();
uint8_t timeout_receiver = uart_base_reg[channel_]->LSR & (1 << 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't polling if you only check once.

1. Consolidated multiple look up tables for the pairs of pins and
added enumerations to simplify the code.
2. Has the timeout feature implemented.
3. Changed pin to pin_ and port to port_ in pin.hpp and gpio.hpp files
4. Working on improving code coverage. The only line that doesn't work
deals with checking the DLL.
@Jeremy-Chau
Copy link
Contributor Author

I should be finished with the code. Just can't figure out why the DLL register is empty from the testing file.

@kammce kammce merged commit 482cad9 into libhal:master Oct 28, 2018
@kammce kammce mentioned this pull request Oct 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants