From 477097f32a3df201ddc531e56127e110896ae13c Mon Sep 17 00:00:00 2001
From: Brendan Bartels <bbartels@iastate.edu>
Date: Sat, 21 Jan 2017 15:54:06 -0600
Subject: [PATCH] quad: Code Review Feedback
- Use u8, u16, and u32 in favor of byte, short, and int
---
quad/sw/modular_quad_pid/src/commands.c | 2 +-
quad/sw/modular_quad_pid/src/communication.c | 14 ++--
quad/sw/modular_quad_pid/src/uart_buff.c | 68 ++++++++++++++------
quad/sw/modular_quad_pid/src/uart_buff.h | 9 ++-
4 files changed, 63 insertions(+), 30 deletions(-)
diff --git a/quad/sw/modular_quad_pid/src/commands.c b/quad/sw/modular_quad_pid/src/commands.c
index c89696f2c..36ce1e9b9 100644
--- a/quad/sw/modular_quad_pid/src/commands.c
+++ b/quad/sw/modular_quad_pid/src/commands.c
@@ -419,7 +419,7 @@ int update(modular_structs_t *structs)
quadPosition_t* currentQuadPosition = &(structs->raw_sensor_struct.currentQuadPosition);
// Packet must come as [NEARPY], 4 bytes each
- int packetId = uart_buff_data_get_int(0);
+ int packetId = uart_buff_data_get_u32(0);
// printf("Packet ID: %d\n", packetId);
float y_pos = uart_buff_data_get_float(4);
// printf("y_pos: %f\n", y_pos);
diff --git a/quad/sw/modular_quad_pid/src/communication.c b/quad/sw/modular_quad_pid/src/communication.c
index 11935385d..2a428909a 100644
--- a/quad/sw/modular_quad_pid/src/communication.c
+++ b/quad/sw/modular_quad_pid/src/communication.c
@@ -70,19 +70,19 @@ int initUartComms() {
int process_packet(modular_structs_t *structs) {
metadata_t meta_data;
// parse metadata
- meta_data.begin_char = uart_buff_get_byte(0);
- meta_data.msg_type = uart_buff_get_byte(01);
- meta_data.msg_subtype = uart_buff_get_byte(2);
- meta_data.msg_id = (uart_buff_get_byte(4) << 8) | (uart_buff_get_byte(3));
- meta_data.data_len = (uart_buff_get_byte(6) << 8) | (uart_buff_get_byte(5));
- unsigned char packet_checksum = uart_buff_get_byte(7+meta_data.data_len);
+ meta_data.begin_char = uart_buff_get_u8(0);
+ meta_data.msg_type = uart_buff_get_u8(1);
+ meta_data.msg_subtype = uart_buff_get_u8(2);
+ meta_data.msg_id = uart_buff_get_u16(3);
+ meta_data.data_len = uart_buff_get_u16(5);
+ unsigned char packet_checksum = uart_buff_get_u8(7+meta_data.data_len);
//unsigned char* packet_data = packet + sizeof(metadata_t);
// Compute checksum
int i;
unsigned char calculated_checksum = 0;
for(i = 0; i < meta_data.data_len + 7; i++){
- calculated_checksum ^= uart_buff_get_byte(i);
+ calculated_checksum ^= uart_buff_get_u8(i);
}
// Discard if checksum didn't match
if(packet_checksum != calculated_checksum) {
diff --git a/quad/sw/modular_quad_pid/src/uart_buff.c b/quad/sw/modular_quad_pid/src/uart_buff.c
index a11a78f47..fab7e464f 100644
--- a/quad/sw/modular_quad_pid/src/uart_buff.c
+++ b/quad/sw/modular_quad_pid/src/uart_buff.c
@@ -66,7 +66,7 @@ void uart_buff_scan_packet_size() {
return;
}
- packet_data_length = (uart_buff_get_byte(6) << 8) | uart_buff_get_byte(5);
+ packet_data_length = (uart_buff_get_u8(6) << 8) | uart_buff_get_u8(5);
packet_size_found = 1;
if (uart_buff_packet_size() > UART_MAX_PACKET_SIZE) {
@@ -87,36 +87,66 @@ int uart_buff_packet_ready() {
}
/**
- * Retrieve a byte from the buffer according to the given offset with respect
+ * Retrieve a 8-bit from the buffer according to the given offset with respect
* to the start index of the buffer.
*/
-u8 uart_buff_get_byte(size_t offset) {
+u8 uart_buff_get_u8(size_t offset) {
size_t target = uart_buff_calc_index(start + offset);
return buff[target];
}
/**
- * Retrieve a byte from the buffer according to the given offset with respect
- * to the start of the data portion of the current packet.
+ * Retrieve a 16-bit from the buffer according to the given offset with respect
+ * to the start index of the buffer.
+ */
+u16 uart_buff_get_u16(size_t offset) {
+ return uart_buff_data_get_u8(offset + 1) << 8 |
+ uart_buff_data_get_u8(offset);
+}
+
+/**
+ * Retrieve a 32-bit from the buffer according to the given offset with respect
+ * to the start index of the buffer.
+ */
+u32 uart_buff_get_u32(size_t offset) {
+ return uart_buff_data_get_u8(offset + 3) << 24 |
+ uart_buff_data_get_u8(offset + 2) << 16 |
+ uart_buff_data_get_u8(offset + 1) << 8 |
+ uart_buff_data_get_u8(offset);
+}
+
+/**
+ * Retrieve a 8-bit unsigned integer from the buffer according to
+ * the given offset with respect to the start of the data portion
+ * of the current packet.
+ *
+ * This has undefined behavior if a packet is not ready.
+ */
+u8 uart_buff_data_get_u8(size_t offset) {
+ return uart_buff_get_u8(7 + offset);
+}
+
+/**
+ * Retrieve a 16-bit unsigned integer from the buffer according to
+ * the given offset with respect to the start of the data portion
+ * of the current packet.
*
* This has undefined behavior if a packet is not ready.
*/
-u8 uart_buff_data_get_byte(size_t offset) {
- return uart_buff_get_byte(7 + offset);
+u16 uart_buff_data_get_u16(size_t offset) {
+ return uart_buff_get_u16(7 + offset);
}
/**
- * Retrieve a 32-bit integer from the buffer according to the givein offset with
- * respect to the start of the data portion of the current packet.
+ * Retrieve a 32-bit unsigned integer from the buffer according to
+ * the given offset with respect to the start of the data portion
+ * of the current packet.
*
* This has undefined behavior if a packet is not ready.
*/
-int uart_buff_data_get_int(size_t offset) {
- return uart_buff_data_get_byte(offset + 3) << 24 |
- uart_buff_data_get_byte(offset + 2) << 16 |
- uart_buff_data_get_byte(offset + 1) << 8 |
- uart_buff_data_get_byte(offset);
+u32 uart_buff_data_get_u32(size_t offset) {
+ return uart_buff_get_u32(7 + offset);
}
/**
@@ -131,10 +161,10 @@ float uart_buff_data_get_float(size_t offset) {
float f;
int i;
} x;
- x.i = uart_buff_data_get_byte(offset + 3) << 24 |
- uart_buff_data_get_byte(offset + 2) << 16 |
- uart_buff_data_get_byte(offset + 1) << 8 |
- uart_buff_data_get_byte(offset);
+ x.i = uart_buff_data_get_u8(offset + 3) << 24 |
+ uart_buff_data_get_u8(offset + 2) << 16 |
+ uart_buff_data_get_u8(offset + 1) << 8 |
+ uart_buff_data_get_u8(offset);
return x.f;
}
@@ -223,7 +253,7 @@ char * uart_buff_get_packet() {
str[packet_size + 1] = '\0'; // null terminate string
int i;
for (i = 0; i < packet_size; i += 1) {
- str[i] = uart_buff_get_byte(i);
+ str[i] = uart_buff_get_u8(i);
}
return str;
}
diff --git a/quad/sw/modular_quad_pid/src/uart_buff.h b/quad/sw/modular_quad_pid/src/uart_buff.h
index 7dcd0fa69..cdf4298f2 100644
--- a/quad/sw/modular_quad_pid/src/uart_buff.h
+++ b/quad/sw/modular_quad_pid/src/uart_buff.h
@@ -8,9 +8,12 @@ void uart_buff_scan_packet();
void uart_buff_scan_packet_start();
void uart_buff_scan_packet_size();
int uart_buff_packet_ready();
-u8 uart_buff_get_byte(size_t);
-u8 uart_buff_data_get_byte(size_t);
-int uart_buff_data_get_int(size_t);
+u8 uart_buff_get_u8(size_t);
+u16 uart_buff_get_u16(size_t);
+u32 uart_buff_get_u32(size_t);
+u8 uart_buff_data_get_u8(size_t);
+u16 uart_buff_data_get_u16(size_t);
+u32 uart_buff_data_get_u32(size_t);
float uart_buff_data_get_float(size_t);
void uart_buff_flush_packet();
size_t uart_buff_size();
--
GitLab