Skip to content

Commit

Permalink
Fixed a stupid bug in the C++ application (#4)
Browse files Browse the repository at this point in the history 
* - Renamed `getChannel` to `getChannelState` because `getChannel` is a stupid name for this function.
 - Fixed a stupid bug where I mixed up GPIOs and channels
 - Reading channels, enabling channels and disabling channels now works as expected.
 - Fixed help text
 - Channel arguments now no longer have optional flags
 - Fixed small logic error in the output text.

* Added new `all` switch

* Added basic toolchain files for CMake

* Bumped up version to next minor
  • Loading branch information
@SimonCahill
SimonCahill authored Aug 27, 2024
1 parent 1f26421 commit 65615d2
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cpp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.20)

project(waveshare_channel_select LANGUAGES CXX VERSION 1.0.0)
project(waveshare_channel_select LANGUAGES CXX VERSION 1.1.0)
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

Expand Down
73 changes: 40 additions & 33 deletions cpp/channel_select.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ using optsm_t = optional<StateModifier>;
optsm_t stateModifierFromString(const string& optArg); //!< Converts an optarg value to a boolean
int32_t parseArgs(int32_t argc, char** argv); //!< Parses arguments passed to the application

bool getChannel(const int32_t channelId); //!< Gets the state of a given channel
bool getChannelState(const int32_t channelId); //!< Gets the state of a given channel
void listChannels(); //!< Lists all channels available with their state.
void setChannel(const int32_t channelId, const StateModifier newState); //!< Sets the state of a given channel

Expand All @@ -111,11 +111,11 @@ int32_t main(int32_t argc, char** argv) {
setChannel(channel, *state);
break;
default:
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channel, CHANNELS[channel], getChannel(CHANNELS[channel]) ? "OFF" : "ON");
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channel + 1, CHANNELS[channel], getChannelState(CHANNELS[channel]) ? "OFF" : "ON");
break;
}
} else if (g_stateModifier == StateModifier::READ) {
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channel, CHANNELS[channel], getChannel(CHANNELS[channel]) ? "OFF" : "ON");
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channel + 1, CHANNELS[channel], getChannelState(CHANNELS[channel]) ? "OFF" : "ON");
continue;
} else {
setChannel(channel, g_stateModifier);
Expand All @@ -135,35 +135,36 @@ int32_t main(int32_t argc, char** argv) {
*/
int32_t parseArgs(int32_t argc, char** argv) {
constexpr option APP_OPTIONS[] = {
{ "help", no_argument, nullptr, 'h' },
{ "version", no_argument, nullptr, 'v' },
{ "help", no_argument, nullptr, 'h' },
{ "version", no_argument, nullptr, 'v' },

// channels
{ "channel1", optional_argument, nullptr, '1' },
{ "channel2", optional_argument, nullptr, '2' },
{ "channel3", optional_argument, nullptr, '3' },
{ "channel4", optional_argument, nullptr, '4' },
{ "channel5", optional_argument, nullptr, '5' },
{ "channel6", optional_argument, nullptr, '6' },
{ "channel7", optional_argument, nullptr, '7' },
{ "channel8", optional_argument, nullptr, '8' },
{ "channel1", no_argument, nullptr, '1' },
{ "channel2", no_argument, nullptr, '2' },
{ "channel3", no_argument, nullptr, '3' },
{ "channel4", no_argument, nullptr, '4' },
{ "channel5", no_argument, nullptr, '5' },
{ "channel6", no_argument, nullptr, '6' },
{ "channel7", no_argument, nullptr, '7' },
{ "channel8", no_argument, nullptr, '8' },
{ "all", no_argument, nullptr, 'a' },

// modifiers
{ "enable", no_argument, nullptr, 'e' },
{ "disable", no_argument, nullptr, 'd' },
{ "read-state", no_argument, nullptr, 'r' },
{ "enable", no_argument, nullptr, 'e' },
{ "disable", no_argument, nullptr, 'd' },
{ "read-state", no_argument, nullptr, 'r' },

// other operations
{ "list-all", no_argument, nullptr, 'L' }
{ "list-all", no_argument, nullptr, 'L' }
};
constexpr const char* APP_SHORTOPTS = R"(hv1::2::3::4::5::6::7::8::edrL)";
constexpr const char* APP_SHORTOPTS = R"(hv12345678edrLa)";

int32_t optChar = -1;

while ((optChar = getopt_long(argc, argv, APP_SHORTOPTS, APP_OPTIONS, nullptr)) != -1) {
if (optChar >= 49 && optChar <= 56) {
// Handle channel inputs
g_channelOptions.try_emplace(CHANNELS[optChar - 49], stateModifierFromString(optarg == nullptr ? string{} : optarg));
g_channelOptions.try_emplace(optChar - 49, std::nullopt);
} else {
switch (optChar) {
case 'h':
Expand All @@ -184,14 +185,19 @@ int32_t parseArgs(int32_t argc, char** argv) {
case 'L':
g_readAll = true;
break;
case 'a':
for (int32_t i = 0; i < 8; i++) {
g_channelOptions.try_emplace(i, std::nullopt);
}
break;
}
}
}

return 0;
}

bool getChannel(const int32_t channel) {
bool getChannelState(const int32_t channel) {
auto gpioChip = gpiod::chip(string{GPIO_CHIP});
auto lineSettings = gpiod::line_settings{};
auto line = gpioChip.prepare_request().add_line_settings(gpioChip.get_line_offset_from_name(fmt::format("GPIO{0:d}", channel)), lineSettings).do_request();
Expand Down Expand Up @@ -221,7 +227,7 @@ void listChannels() {
size_t channelCounter = 1;

for (const auto channel : CHANNELS) {
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channelCounter++, channel, getChannel(channel) ? "OFF" : "ON");
fmt::println("Channel {0:d} (GPIO pin {1:d}) set to {2:s}", channelCounter++, channel, getChannelState(channel) ? "OFF" : "ON");
}
}

Expand All @@ -232,7 +238,7 @@ void printHelp() {
Usage:
{0:s} -h
{0:s} -e -123 # enable channels 1, 2, and 3
{0:s} -d -528 -7=r # disable channels 5, 2, and 8 and read the state of channel 7
{0:s} -d -528 # disable channels 5, 2, and 8 and read the state of channel 7
{0:s} -L # read the states of all channels
Troubleshooting:
Expand All @@ -241,21 +247,22 @@ void printHelp() {
Options:
Channel selection:
--channel1, -1[=edr] Channel 1
--channel2, -2[=edr] Look, it's the same up until -8
--channel1, -1 Channel 1
--channel2, -2 Look, it's the same up until -8
...
--channel8, -8[=edr] I'm sure it's self-explanatory at this point
--channel8, -8 I'm sure it's self-explanatory at this point
--all, -a All channels
Channel options:
--enable, -e Enable channel(s)
--disable, -d Disable channel(s)
--read, -r Read the channel state
--enable, -e Enable channel(s)
--disable, -d Disable channel(s)
--read, -r Read the channel state
General options:
--list-all, -L List all channels and their current state
--list-all, -L List all channels and their current state
--help, -h Displays this text and exits
--version, -v Displays the version information and exits
--help, -h Displays this text and exits
--version, -v Displays the version information and exits
)", APP_NAME, APP_VERS);

}
Expand All @@ -265,13 +272,13 @@ void printVersion() {
}

void setChannel(const int32_t channel, const StateModifier newState) {
const auto gpioLineName = fmt::format("GPIO{0:d}", channel);
const auto gpioLineName = fmt::format("GPIO{0:d}", CHANNELS[channel]);

auto gpioChip = gpiod::chip(string{GPIO_CHIP});
auto lineSettings = gpiod::line_settings{};
lineSettings.set_direction(gpiod::line::direction::OUTPUT);
auto line = gpioChip.prepare_request().add_line_settings(gpioChip.get_line_offset_from_name(gpioLineName), lineSettings).do_request();

fmt::println("Attempting to set GPIO{0:d} {1:s}", channel, newState == StateModifier::DISABLE ? "OFF" : "ON");
fmt::println("Attempting to set GPIO{0:d} {1:s}", CHANNELS[channel], newState == StateModifier::DISABLE ? "ON" : "OFF");
line.set_value(line.offsets()[0], newState == StateModifier::DISABLE ? gpiod::line::value::INACTIVE : gpiod::line::value::ACTIVE);
}
18 changes: 18 additions & 0 deletions cpp/toolchains/aarch64-toolchain.cmake
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Set the CMake cross-compiling toolchain
set(CMAKE_SYSTEM_NAME Linux)
set(CMAKE_SYSTEM_PROCESSOR arm)

# Set the path to the aarch64-linux-gnu toolchain
set(CMAKE_C_COMPILER aarch64-linux-gnu-gcc)
set(CMAKE_CXX_COMPILER aarch64-linux-gnu-g++)

# Set the target architecture
set(CMAKE_LIBRARY_ARCHITECTURE aarch64-linux-gnu)

# Specify the linker
set(CMAKE_LINKER aarch64-linux-gnu-ld)

# Specify the necessary libraries and include directories
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
22 changes: 22 additions & 0 deletions cpp/toolchains/armhf-toolchain.cmake
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Set the CMake cross-compiling toolchain
set(CMAKE_SYSTEM_NAME Linux)
set(CMAKE_SYSTEM_PROCESSOR arm)

# Set the path to the arm-linux-gnueabihf toolchain
set(CMAKE_C_COMPILER arm-linux-gnueabihf-gcc)
set(CMAKE_CXX_COMPILER arm-linux-gnueabihf-g++)

# Set the target architecture
set(CMAKE_LIBRARY_ARCHITECTURE arm-linux-gnueabihf)

# Set other necessary flags and options
set(CMAKE_C_FLAGS "-march=armv7-a -mfpu=neon -mfloat-abi=hard")
set(CMAKE_CXX_FLAGS "-march=armv7-a -mfpu=neon -mfloat-abi=hard")

# Specify the linker
set(CMAKE_LINKER arm-linux-gnueabihf-ld)

# Specify the necessary libraries and include directories
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

0 comments on commit 65615d2

Please sign in to comment.