From 5fdbad0d0c6ed465dce71b928363dca9b305a07d Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 13 Jan 2022 16:29:45 -0500 Subject: [PATCH] Fix #94, move all bitfields into codec Nothing outside of the codec code should need to know about the bitfields in the CFDP protocol definition. Everything else uses logical values. This moves all DECLARE_FIELD macros into cf_codec.c and removes cf_field.h. --- fsw/src/cf_cfdp_pdu.h | 53 --------------- fsw/src/cf_codec.c | 102 +++++++++++++++++++++++++++-- fsw/src/cf_field.h | 71 -------------------- unit-test/cf_cfdp_dispatch_tests.c | 1 - unit-test/cf_cfdp_r_tests.c | 1 - unit-test/cf_cfdp_s_tests.c | 1 - 6 files changed, 96 insertions(+), 133 deletions(-) delete mode 100644 fsw/src/cf_field.h diff --git a/fsw/src/cf_cfdp_pdu.h b/fsw/src/cf_cfdp_pdu.h index e2aa29505..dd94cce6d 100644 --- a/fsw/src/cf_cfdp_pdu.h +++ b/fsw/src/cf_cfdp_pdu.h @@ -41,7 +41,6 @@ #include "common_types.h" #include "cf_platform_cfg.h" -#include "cf_field.h" #include "cf_platform_cfg.h" #include @@ -146,22 +145,6 @@ typedef struct CF_CFDP_PduHeader } CF_CFDP_PduHeader_t; -/* - * Fields within the "flags" byte of the PDU header - */ -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_VERSION, 3, 5) -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_TYPE, 1, 4) -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_DIR, 1, 3) -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_MODE, 1, 2) -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_CRC, 1, 1) -DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_LARGEFILE, 1, 0) - -/* - * Fields within the "eid_tsn_lengths" byte of the PDU header - */ -DECLARE_FIELD(CF_CFDP_PduHeader_LENGTHS_ENTITY, 3, 4) -DECLARE_FIELD(CF_CFDP_PduHeader_LENGTHS_TRANSACTION_SEQUENCE, 3, 0) - /** * @brief Structure representing CFDP File Directive Header * @@ -336,11 +319,6 @@ typedef struct CF_CFDP_PduEof } CF_CFDP_PduEof_t; -/* - * Position of the condition code value within the CC field - */ -DECLARE_FIELD(CF_CFDP_PduEof_FLAGS_CC, 4, 4) - /** * @brief Structure representing CFDP Finished PDU * @@ -352,13 +330,6 @@ typedef struct CF_CFDP_PduFin } CF_CFDP_PduFin_t; -/* - * Position of the sub-field values within the flags field - */ -DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_CC, 4, 4) -DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_DELIVERY_CODE, 1, 2) -DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_FILE_STATUS, 2, 0) - /** * @brief Structure representing CFDP Acknowledge PDU * @@ -370,15 +341,6 @@ typedef struct CF_CFDP_PduAck CF_CFDP_uint8_t cc_and_transaction_status; } CF_CFDP_PduAck_t; -/* - * Position of the sub-field values within the directive_and_subtype_code - * and cc_and_transaction_status fields within the ACK PDU. - */ -DECLARE_FIELD(CF_CFDP_PduAck_DIR_CODE, 4, 4) -DECLARE_FIELD(CF_CFDP_PduAck_DIR_SUBTYPE_CODE, 4, 0) -DECLARE_FIELD(CF_CFDP_PduAck_CC, 4, 4) -DECLARE_FIELD(CF_CFDP_PduAck_TRANSACTION_STATUS, 2, 0) - /** * @brief Structure representing CFDP Segment Request * @@ -414,13 +376,6 @@ typedef struct CF_CFDP_PduMd } CF_CFDP_PduMd_t; -/* - * Position of the sub-field values within the directive_and_subtype_code - * and cc_and_transaction_status fields within the ACK PDU. - */ -DECLARE_FIELD(CF_CFDP_PduMd_CLOSURE_REQUESTED, 1, 7) -DECLARE_FIELD(CF_CFDP_PduMd_CHECKSUM_TYPE, 4, 0) - typedef struct CF_CFDP_PduFileDataHeader { /* @@ -431,14 +386,6 @@ typedef struct CF_CFDP_PduFileDataHeader CF_CFDP_uint32_t offset; } CF_CFDP_PduFileDataHeader_t; -/* - * Position of the optional sub-field values within the file data PDU header - * These are present only if the "segment metadata" flag in the common header - * is set to 1. - */ -DECLARE_FIELD(CF_CFDP_PduFileData_RECORD_CONTINUATION_STATE, 2, 6) -DECLARE_FIELD(CF_CFDP_PduFileData_SEGMENT_METADATA_LENGTH, 6, 0) - /* * To serve as a sanity check, this should accommodate the largest data block possible. * In that light, it should be sized based on the minimum encoded header size, rather than diff --git a/fsw/src/cf_codec.c b/fsw/src/cf_codec.c index fc84a5e1b..37670b31d 100644 --- a/fsw/src/cf_codec.c +++ b/fsw/src/cf_codec.c @@ -24,12 +24,100 @@ * CFDP protocol data structure encode/decode implementation */ -#define CF_DO_DECLARE_FIELDS - #include "cf_cfdp_pdu.h" #include "cf_codec.h" #include "cf_events.h" +#define xstr(s) str(s) +#define str(s) #s + +#include + +typedef struct CF_Codec_BitField +{ + uint32 shift; + uint32 mask; +} CF_Codec_BitField_t; + +/* NBITS == number of bits */ +#define DECLARE_FIELD(NAME, NBITS, SHIFT) \ + static const CF_Codec_BitField_t NAME = {.shift = (SHIFT), .mask = ((1 << NBITS) - 1)}; + +/* + * All CFDP sub-fields are fewer than 8 bits in size + */ +static inline uint8 CF_FieldGetVal(const uint8 *src, uint8 shift, uint8 mask) +{ + return (*src >> shift) & mask; +} + +static inline void CF_FieldSetVal(uint8 *dest, uint8 shift, uint8 mask, uint8 val) +{ + *dest &= ~(mask << shift); + *dest |= ((val & mask) << shift); +} + +/* FGV, FSV, and FAV are just simple shortenings of the field macros. + * + * FGV == field get val + * FSV == field set val + */ + +#define FGV(SRC, NAME) CF_FieldGetVal((SRC).octets, (NAME).shift, (NAME).mask) +#define FSV(DEST, NAME, VAL) CF_FieldSetVal((DEST).octets, (NAME).shift, (NAME).mask, VAL) + +/* + * Fields within the "flags" byte of the PDU header + */ +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_VERSION, 3, 5) +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_TYPE, 1, 4) +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_DIR, 1, 3) +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_MODE, 1, 2) +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_CRC, 1, 1) +DECLARE_FIELD(CF_CFDP_PduHeader_FLAGS_LARGEFILE, 1, 0) + +/* + * Fields within the "eid_tsn_lengths" byte of the PDU header + */ +DECLARE_FIELD(CF_CFDP_PduHeader_LENGTHS_ENTITY, 3, 4) +DECLARE_FIELD(CF_CFDP_PduHeader_LENGTHS_TRANSACTION_SEQUENCE, 3, 0) + +/* + * Position of the condition code value within the CC field for EOF + */ +DECLARE_FIELD(CF_CFDP_PduEof_FLAGS_CC, 4, 4) + +/* + * Position of the sub-field values within the flags field for FIN + */ +DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_CC, 4, 4) +DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_DELIVERY_CODE, 1, 2) +DECLARE_FIELD(CF_CFDP_PduFin_FLAGS_FILE_STATUS, 2, 0) + +/* + * Position of the sub-field values within the directive_and_subtype_code + * and cc_and_transaction_status fields within the ACK PDU. + */ +DECLARE_FIELD(CF_CFDP_PduAck_DIR_CODE, 4, 4) +DECLARE_FIELD(CF_CFDP_PduAck_DIR_SUBTYPE_CODE, 4, 0) +DECLARE_FIELD(CF_CFDP_PduAck_CC, 4, 4) +DECLARE_FIELD(CF_CFDP_PduAck_TRANSACTION_STATUS, 2, 0) + +/* + * Position of the sub-field values within the directive_and_subtype_code + * and cc_and_transaction_status fields within the ACK PDU. + */ +DECLARE_FIELD(CF_CFDP_PduMd_CLOSURE_REQUESTED, 1, 7) +DECLARE_FIELD(CF_CFDP_PduMd_CHECKSUM_TYPE, 4, 0) + +/* + * Position of the optional sub-field values within the file data PDU header + * These are present only if the "segment metadata" flag in the common header + * is set to 1. + */ +DECLARE_FIELD(CF_CFDP_PduFileData_RECORD_CONTINUATION_STATE, 2, 6) +DECLARE_FIELD(CF_CFDP_PduFileData_SEGMENT_METADATA_LENGTH, 6, 0) + /* NOTE: get/set will handle endianess */ /* * ALSO NOTE: These store/set inline functions/macros are used with @@ -753,10 +841,12 @@ void CF_CFDP_DecodeHeader(CF_DecoderState_t *state, CF_Logical_PduHeader_t *plh) peh = CF_DECODE_FIXED_CHUNK(state, CF_CFDP_PduHeader_t); if (peh != NULL) { - plh->version = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_VERSION); - plh->direction = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_DIR); - plh->pdu_type = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_TYPE); - plh->txm_mode = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_MODE); + plh->version = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_VERSION); + plh->direction = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_DIR); + plh->pdu_type = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_TYPE); + plh->txm_mode = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_MODE); + plh->crc_flag = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_CRC); + plh->large_flag = FGV(peh->flags, CF_CFDP_PduHeader_FLAGS_LARGEFILE); /* The eid+tsn lengths are encoded as -1 */ plh->eid_length = FGV(peh->eid_tsn_lengths, CF_CFDP_PduHeader_LENGTHS_ENTITY) + 1; diff --git a/fsw/src/cf_field.h b/fsw/src/cf_field.h deleted file mode 100644 index 047708ae2..000000000 --- a/fsw/src/cf_field.h +++ /dev/null @@ -1,71 +0,0 @@ -/************************************************************************ - * - * NASA Docket No. GSC-18,447-1, and identified as “CFS CFDP (CF) - * Application version 3.0.0” - * Copyright © 2019 United States Government as represented by the - * Administrator of the National Aeronautics and Space Administration. - * All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. You may obtain - * a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - ************************************************************************/ - -/** - * @file - * - * The CF Application field macro header file - */ - -#ifndef CF_FIELD_COMMON_H -#define CF_FIELD_COMMON_H - -#define xstr(s) str(s) -#define str(s) #s - -#include - -typedef struct CF_FIELD_FIELD -{ - uint32 shift; - uint32 mask; -} CF_FIELD_FIELD; - -/* NBITS == number of bits */ -#ifdef CF_DO_DECLARE_FIELDS -#define DECLARE_FIELD(NAME, NBITS, SHIFT) \ - static const CF_FIELD_FIELD NAME = {.shift = (SHIFT), .mask = ((1 << NBITS) - 1)}; -#else -#define DECLARE_FIELD(NAME, NBITS, SHIFT) -#endif - -/* - * All CFDP sub-fields are fewer than 8 bits in size - */ -static inline uint8 CF_FieldGetVal(const uint8 *src, uint8 shift, uint8 mask) -{ - return (*src >> shift) & mask; -} - -static inline void CF_FieldSetVal(uint8 *dest, uint8 shift, uint8 mask, uint8 val) -{ - *dest &= ~(mask << shift); - *dest |= ((val & mask) << shift); -} - -/* FGV, FSV, and FAV are just simple shortenings of the field macros. - * - * FGV == field get val - * FSV == field set val - */ - -#define FGV(SRC, NAME) CF_FieldGetVal((SRC).octets, (NAME).shift, (NAME).mask) -#define FSV(DEST, NAME, VAL) CF_FieldSetVal((DEST).octets, (NAME).shift, (NAME).mask, VAL) - -#endif /* !CF_FIELD_COMMON_H */ diff --git a/unit-test/cf_cfdp_dispatch_tests.c b/unit-test/cf_cfdp_dispatch_tests.c index 23584e3ac..98c75eced 100644 --- a/unit-test/cf_cfdp_dispatch_tests.c +++ b/unit-test/cf_cfdp_dispatch_tests.c @@ -3,7 +3,6 @@ #include "cf_test_alt_handler.h" #include "cf_cfdp.h" #include "cf_app.h" -#include "cf_field.h" #include "cf_events.h" #include "cf_cfdp_r.h" diff --git a/unit-test/cf_cfdp_r_tests.c b/unit-test/cf_cfdp_r_tests.c index a2e409a64..ffb806284 100644 --- a/unit-test/cf_cfdp_r_tests.c +++ b/unit-test/cf_cfdp_r_tests.c @@ -3,7 +3,6 @@ #include "cf_test_alt_handler.h" #include "cf_cfdp.h" #include "cf_app.h" -#include "cf_field.h" #include "cf_events.h" #include "cf_cfdp_r.h" diff --git a/unit-test/cf_cfdp_s_tests.c b/unit-test/cf_cfdp_s_tests.c index 93ea1a0fc..0ea9805ee 100644 --- a/unit-test/cf_cfdp_s_tests.c +++ b/unit-test/cf_cfdp_s_tests.c @@ -3,7 +3,6 @@ #include "cf_test_alt_handler.h" #include "cf_cfdp.h" #include "cf_app.h" -#include "cf_field.h" #include "cf_events.h" #include "cf_cfdp_r.h"