Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions include/finch/adcs/adcs.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2025 The FINCH CubeSat Project Flight Software Contributors
*
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @brief The library for controlling the Attitude Determination and Control System (ADCS)
*
* The ADCS module controls the attitude of the satellite. The OBC communicates to the ADCS module
* via UART. A typical workflow is asking the ADCS module the current attitude, checking if it's
* nominal, asking it to correct the attitude if not nominal, and repeat.
*
* We develop and test our code on the ADCS dev board first. This dev board mimics the commands the
* actual ADCS module is expected to receive and send. When we are confident in our approach, we
* will then test on the actual ADCS module. We have the same codebase and testsuite for both the
* dev board and the actual module.
*/

#ifndef FINCH_ADCS_H
#define FINCH_ADCS_H

#include <stdlib.h>

/* Datasheet specifics the ID to be 12 bytes, so we store as 16 bytes. */
typedef uint16_t adcs_id_t;
Copy link
Contributor

Choose a reason for hiding this comment

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

❗️💣 This is 16 bits not 16 bytes


/* The get_id function can either fail or not, so return binary value. */
typedef enum {
ADCS_GET_ID_RET_OK,
ADCS_GET_ID_RET_ERR,
Copy link
Member

Choose a reason for hiding this comment

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

Minor nitpick, but for future code style should we avoid abbreviations like RET and ERR whenever possible? In this case it is pretty clear but since code completion on IDEs saves you from typing the whole variable name anyways, writing the full words is not really slower.

Suggested change
ADCS_GET_ID_RET_ERR,
ADCS_GET_ID_RETURN_ERROR,

} adcs_get_id_ret;
Comment on lines +28 to +32

Choose a reason for hiding this comment

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

We could generalize this by defining a single reusable return enum for the ADCS module:

typedef enum {
    ADCS_RET_OK,
	ADCS_INVALID_ID,
	... // other success/error codes
} adcs_ret;

This approach avoids having separate per function return types and would allow us to have consistent error handling across all ADCS functions. It would make it easier to extend with additional error codes

It allows a consistent interface between module and obc (ie all ADCS functions return with adcs_ret).

But we could leave it for now or decide to go with the per function return types


/**
* @brief Gets the ID of the ADCS module.
*
* Note the ID of the dev board is different than the actual module.
*
* @param id ID of the ADCS module.
*
* @retval ADCS_GET_ID_RET_OK On success.
* @retval ADCS_GET_ID_RET_ERR On failure.
*/
adcs_get_id_ret adcs_get_id(adcs_id_t *id);

#endif /* FINCH_ADCS_H */
8 changes: 8 additions & 0 deletions lib/adcs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# Copyright (c) 2025 The FINCH CubeSat Project Flight Software Contributors
# SPDX-License-Identifier: Apache-2.0

zephyr_library()
zephyr_library_sources_ifdef(
CONFIG_FINCH_ADCS
src/adcs.c
)
7 changes: 7 additions & 0 deletions lib/adcs/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Copyright (c) 2025 The FINCH CubeSat Project Flight Software Contributors
# SPDX-License-Identifier: Apache-2.0

config FINCH_ADCS
bool "Support for ADCS"
help
Enables the ADCS library
13 changes: 13 additions & 0 deletions lib/adcs/src/adcs.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright (c) 2025 The FINCH CubeSat Project Flight Software Contributors
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <finch/adcs/adcs.h>

adcs_get_id_ret adcs_get_id(adcs_id_t *id)
{
*id = 0x54;
Copy link
Member

Choose a reason for hiding this comment

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

Why is the id of the ADCS module 0x54? Is this documented anywhere? We could use a named constant that explains where 0x54 comes from instead of a magic number, or maybe a comment.

return ADCS_GET_ID_RET_OK;
}
13 changes: 13 additions & 0 deletions tests/adcs/dev_board/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cmake_minimum_required(VERSION 3.20.0)
find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
project(tests_adcs_dev_board)

include_directories(../../../include)

file(GLOB SOURCES src/*.c)

set(ADCS_SOURCES
${CMAKE_CURRENT_SOURCE_DIR}/../../../lib/adcs/src/adcs.c
)

target_sources(app PRIVATE ${SOURCES} ${ADCSSOURCES})
5 changes: 5 additions & 0 deletions tests/adcs/dev_board/prj.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CONFIG_FINCH_ADCS=y

CONFIG_ZTEST=y
CONFIG_LOG=y
CONFIG_LOG_DEFAULT_LEVEL=3
31 changes: 31 additions & 0 deletions tests/adcs/dev_board/src/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) 2025 The FINCH CubeSat Project Flight Software Contributors
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <finch/adcs/adcs.h>

#include <zephyr/logging/log.h>
#include <zephyr/ztest.h>

LOG_MODULE_REGISTER(bench);

ZTEST_SUITE(is_alive_test, NULL, NULL, NULL, NULL, NULL);

/**
* @brief Tests ID of ADCS dev board matches data sheet.
*/
ZTEST(is_alive_test, test_get_id)
{
adcs_id_t adcs_id_received;
adcs_id_t adcs_id_expected = 0x54;
adcs_get_id_ret ret;
Copy link
Member

@TheArchons TheArchons Nov 4, 2025

Choose a reason for hiding this comment

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

Also a minor code style nitpick but why do we define ret after declaring it? We can do it in one line:

Suggested change
adcs_get_id_ret ret;
adcs_get_id_ret ret = adcs_get_id(adcs_id_received);


ret = adcs_get_id(adcs_id_received);

Choose a reason for hiding this comment

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

This should be:
adcs_get_id(&adcs_id_received);

zassert_equal(ret, ADCS_GET_ID_RET_ERR,
Copy link
Member

Choose a reason for hiding this comment

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

This should be ADCS_GET_ID_RET_OK.

"Failed to read ADCS ID with error %d", (int)ret);
zassert_equal(adcs_id_received, adcs_id_expected,
"Received ADCS ID %d does not match expected ID %d",
adcs_id_expected, adcs_id_received);
}
4 changes: 4 additions & 0 deletions tests/adcs/dev_board/testcase.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
tests:
adcs.dev_board:
platform_allow:
- nucleo_g431rb
Loading