Using Claude to improve unit tests and implement ICMP

AGENTS.md

@@ -0,0 +1,50 @@
+---
+name: unittest_agent
+description: Expert in developing useful unit tests for embedded development.
+---
+
+# AGENTS.md
+
+## Your Role
+
+- You write for a developer audiences and favor clarity and practical examples.
+- You task is is to make detailed unit tests to cover every path.
+
+## Project Knowledge
+
+You are an Expert Embedded Programmer with experience in:
+
+- 32 bit Cortex-M microcontrollers
+- Low level operating system primitives.
+- Safety oriented versions of C23 or later, following MISRA C guidelines, or similar guidelines. C23 is preferred, when possible.
+- Unit testing low level code through the use of Unity.
+- Writing and using CMake 4.0+ build systems.
+
+## Commands to use
+
+Build and Run Unit Tests for all local compilers:
+
+```bash
+cmake -B build -S . -DCMAKE_INSTALL_PREFIX=../install
+cmake --build build --target all
+cmake --build build --target install
+cmake --build build --target test
+```
+
+## Project Structure
+
+- `source/` - Source code that you read
+- `include/` - Header code that you read
+- `tests/` - Test that you generate for unit testing the source code
+- `documentation/` - Doxygen is used to generate the documentation.
+- `examples/` - Example code that you read to understand how to use the library.
+
+## Boundaries
+
+- ✅ **Always do** make unit tests for new code within the same project. Prefer `unity`. Run clang format on all new code. Finally before a commit, bump the version number following the Semantic Versioning guidelines. Use `tbump` to assist with this and follow it's patterns (ChangeLog.md, README.md, etc).
+- ✅ **Always do** follow the existing coding style and patterns.
+- ✅ **Always do** write clear and concise commit messages that explain the changes made.
+- ✅ **Always do** write clear and concise documentation for any new features or changes including updating README.md and adding Doxygen comments.
+- ✅ **Always do** write unit tests that cover all new code and edge cases.
+- ⚠️ **Ask First** before modifying source code or documentation in a major way.
+- 🚫 **NEVER** modify the git repository or the .git folder.

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.31.0)
 
-project(HyphaIp LANGUAGES C VERSION 0.2.0)
+project(HyphaIp LANGUAGES C VERSION 0.3.0)
 enable_testing()
 include(FetchContent)
 
@@ -11,6 +11,9 @@ option(BUILD_UNIT_TESTS "Builds the unit tests" ON)
 option(BUILD_COVERAGE "Builds with coverage support" ON)
 option(BUILD_ANALYSIS "Builds with static analysis support" ON)
 
+# Feature support through cmake options
+option(USE_ICMP "Enable ICMP support" ON)
+
 ###############################################################
 # Interface Libraries
 ###############################################################
@@ -19,6 +22,7 @@ target_compile_definitions(hypha-ip-defs INTERFACE
     HYPHA_IP_TTL=128
     HYPHA_IP_MTU=1500
     $<$<BOOL:${BUILD_UNIT_TESTS}>:HYPHA_IP_UNIT_TEST=1>
+    $<$<BOOL:${USE_ICMP}>:HYPHA_IP_USE_ICMP=1>
 )
 
 add_library(hypha-ip-rules INTERFACE)

ChangeLog.md

@@ -1,5 +1,10 @@
 # Versions
 
+## v0.3.0
+
+* Added ICMP support behind a compile time flag.
+* Improved unit tests to cover error handling paths.
+
 ## v0.2.0
 
 * Adding CMake installation process

README.md

@@ -48,6 +48,7 @@ Users can statically configure Hypha IP in several regards
 * ARP Cache (define `HYPHA_IP_USE_ARP_CACHE` as 1 or 0) and ARP Cache Size (`HYPHA_IP_ARP_TABLE_SIZE` set to a number > 0)
 * IPv4 Checksum Enablement (`HYPHA_IP_USE_IP_CHECKSUM` set to `true` or `false`)
 * UDP Checksum Enablement (`HYPHA_IP_USE_UDP_CHECKSUM` set to `true` or `false`)
+* ICMP Support (define `HYPHA_IP_USE_ICMP` set to 1 or 0)
 * Ethernet MAC Filter (define `HYPHA_IP_USE_MAC_FILTER` to 1 or 0) and number of Filter Elements (`HYPHA_IP_MAC_FILTER_TABLE_SIZE` set to a number > 0)
 * Allow any IP Localhost into the stack (define `HYPHA_IP_ALLOW_ANY_LOCALHOST` to 1 or 0)
 * Allow any IP Broadcast into the stack (define `HYPHA_IP_ALLOW_ANY_BROADCAST` to 1 or 0)

examples/hypha_ip_lifecycle.c

@@ -145,9 +145,10 @@ int main(int argc, char *argv[argc]) {
         // check if the transmit was successful
 
         /// @cond USE_ICMP
-#if defined(HYPHA_IP_USE_ICMP) || defined(HYPHA_IP_USE_ICMPv6)
-        // Transmit ICMP datagrams as needed
-        status = HyphaIpTransmitIcmpDatagram(context, HyphaIpIcmpTypeEchoRequest, 0, &metadata, datagram);
+#if defined(HYPHA_IP_USE_ICMP)
+        // Transmit ICMP echo request (ping)
+        static uint16_t ping_sequence = 0;
+        status = HyphaIpTransmitIcmpEchoRequest(context, metadata.destination_address, 0x1234, ping_sequence++, 56);
 #endif
         /// @endcond
     }

include/hypha_ip/hypha_ip.h

@@ -707,6 +707,18 @@ HyphaIpStatus_e HyphaIpRunOnce(HyphaIpContext_t context);
 HyphaIpStatus_e HyphaIpTransmitUdpDatagram(HyphaIpContext_t context, HyphaIpMetaData_t *metadata,
                                            HyphaIpSpan_t datagram);
 
+#if defined(HYPHA_IP_USE_ICMP)
+/// Transmits an ICMP Echo Request (ping) now.
+/// @param[in] context The opaque context
+/// @param[in] destination The destination IPv4 address
+/// @param[in] identifier The ICMP identifier (typically process ID)
+/// @param[in] sequence The ICMP sequence number
+/// @param[in] data_size The size of data payload (max 56 bytes)
+/// @return The status of the operation
+HyphaIpStatus_e HyphaIpTransmitIcmpEchoRequest(HyphaIpContext_t context, HyphaIpIPv4Address_t destination,
+                                               uint16_t identifier, uint16_t sequence, size_t data_size);
+#endif
+
 /// Gets the statistics of the Hypha IP Stack
 /// @param[in] context The opaque context
 /// @return The statistics of the Hypha IP Stack

source/hypha_arp.c

@@ -17,6 +17,21 @@ HyphaIpStatus_e HyphaIpArpAnnouncement(HyphaIpContext_t context) {
     if (frame == nullptr) {
         return HyphaIpStatusOutOfMemory;
     }
+
+    // Set up the Ethernet header for ARP
+    HyphaIpEthernetHeader_t ethernet_header = {
+        .destination = hypha_ip_ethernet_broadcast,
+        .source = context->interface.mac,
+#if (HYPHA_IP_USE_VLAN == 1)
+        .tpid = HyphaIpEtherType_VLAN,
+        .priority = 0,
+        .drop_eligible = 0,
+        .vlan = HYPHA_IP_VLAN_ID,
+#endif
+        .type = HyphaIpEtherType_ARP,
+    };
+    HyphaIpCopyEthernetHeaderToFrame(frame, &ethernet_header);
+
     HyphaIpArpPacket_t arp_packet = {
         .hardware_type = HyphaIpArpHardwareTypeEthernet,
         .protocol_type = HyphaIpArpProtocolTypeIPv4,
@@ -41,6 +56,12 @@ HyphaIpStatus_e HyphaIpArpAnnouncement(HyphaIpContext_t context) {
 
 HyphaIpStatus_e HyphaIpArpProcessPacket(HyphaIpContext_t context, HyphaIpEthernetFrame_t *frame,
                                         HyphaIpTimestamp_t timestamp) {
+    if (context == nullptr) {
+        return HyphaIpStatusInvalidContext;
+    }
+    if (frame == nullptr) {
+        return HyphaIpStatusInvalidArgument;
+    }
     // TODO the lengths must match 48 bit address and IPv4 address
     context->statistics.counter.arp.rx.count++;
     context->statistics.counter.arp.rx.bytes += sizeof(HyphaIpArpPacket_t);

source/hypha_flip.c

@@ -66,8 +66,11 @@ HYPHA_INTERNAL const HyphaIpFlipUnit_t flip_ip_header[] = {
     {sizeof(uint8_t), 2}, {sizeof(uint16_t), 3}, {sizeof(uint8_t), 2}, {sizeof(uint16_t), 1}, {sizeof(uint8_t), 8},
 };
 
-/// Outlines the flipping units for ICMP headers
-HYPHA_INTERNAL const HyphaIpFlipUnit_t flip_icmp_header[] = {{sizeof(uint16_t), 2}};
+/// Outlines the flipping units for ICMP headers (type and code are uint8_t, only checksum needs flipping)
+HYPHA_INTERNAL const HyphaIpFlipUnit_t flip_icmp_header[] = {{sizeof(uint8_t), 2}, {sizeof(uint16_t), 1}};
+
+/// Outlines the flipping units for ICMP echo packet (identifier and sequence only, not data)
+HYPHA_INTERNAL const HyphaIpFlipUnit_t flip_icmp_echo[] = {{sizeof(uint16_t), 2}};
 
 /// Outlines the flipping units for UDP headers
 HYPHA_INTERNAL const HyphaIpFlipUnit_t flip_udp_header[] = {{sizeof(uint16_t), 4}};
@@ -163,6 +166,26 @@ void HyphaIpCopyIcmpDatagramToFrame(HyphaIpEthernetFrame_t *dst, uint8_t const *
     HyphaIpFlipCopy(HYPHA_IP_DIMOF(flip_udp_header), flip_udp_header, &dst->payload[offset], src);
 }
 
+void HyphaIpCopyIcmpEchoFieldsFromFrame(uint16_t *identifier, uint16_t *sequence, HyphaIpEthernetFrame_t *src) {
+    size_t offset = sizeof(HyphaIpIPv4Header_t) + sizeof(HyphaIpICMPHeader_t);
+    struct {
+        uint16_t identifier;
+        uint16_t sequence;
+    } echo_fields;
+    HyphaIpFlipCopy(HYPHA_IP_DIMOF(flip_icmp_echo), flip_icmp_echo, &echo_fields, &src->payload[offset]);
+    *identifier = echo_fields.identifier;
+    *sequence = echo_fields.sequence;
+}
+
+void HyphaIpCopyIcmpEchoFieldsToFrame(HyphaIpEthernetFrame_t *dst, uint16_t identifier, uint16_t sequence) {
+    size_t offset = sizeof(HyphaIpIPv4Header_t) + sizeof(HyphaIpICMPHeader_t);
+    struct {
+        uint16_t identifier;
+        uint16_t sequence;
+    } echo_fields = {.identifier = identifier, .sequence = sequence};
+    HyphaIpFlipCopy(HYPHA_IP_DIMOF(flip_icmp_echo), flip_icmp_echo, &dst->payload[offset], &echo_fields);
+}
+
 void HyphaIpCopyArpPacketFromFrame(HyphaIpArpPacket_t *dst, HyphaIpEthernetFrame_t *src) {
     HyphaIpFlipCopy(HYPHA_IP_DIMOF(flip_arp_packet), flip_arp_packet, dst, src->payload);
 }