improved packet serialization and readability

CMakeLists.txt

@@ -25,9 +25,9 @@ add_executable(M2-PT-DRP
         source/utils/audio.hpp
         source/EventManager.cpp
         source/EventManager.hpp
-        source/packets/base/GenericPacket.hpp
+        source/packets/base/Packet.hpp
         source/events/types.hpp
-        source/packets/base/GenericPacket.cpp
+        source/packets/base/Packet.cpp
         source/RemotePeer.hpp
         source/events/base/BaseEvent.hpp
         source/events/base/BaseEvent.hpp
@@ -52,6 +52,11 @@ add_executable(M2-PT-DRP
         source/Context.hpp
         source/utils/CacheMap.hpp
         source/packets/search/SearchPacketData.hpp
+        source/packets/base/PacketContent.cpp
+        source/packets/base/PacketContent.hpp
+        source/packets/base/SecurityMode.hpp
+        source/packets/base/PacketData.hpp
+        source/packets/info/InfoPacketData.hpp
 )
 target_include_directories(M2-PT-DRP PRIVATE
         ${MPG123_INCLUDE_DIRS}

source/EventManager.cpp

@@ -16,7 +16,7 @@
 #include "events/info/InfoEvent.hpp"
 #include "events/pong/PongEvent.hpp"
 #include "events/search/SearchEvent.hpp"
-#include "packets/base/GenericPacket.hpp"
+#include "packets/base/Packet.hpp"
 #include "tasks/client/ClientTask.hpp"
 #include "tasks/server/ServerTask.hpp"
 #include "tasks/undefined/UndefinedTask.hpp"
@@ -156,8 +156,8 @@ void EventManager::loopReceiver() {
     // prepare space for the sender address
     sockaddr_storage fromAddress {};
     socklen_t fromAddressLength = sizeof(fromAddress);
-    drp::packet::GenericPacket packet {};
-    drp::packet::GenericPacketContent packetContent {};
+    drp::packet::base::Packet packet {};
+    drp::packet::base::PacketContent packetContent {};
 
     // client loop
     while (true) {

source/events/audio/AudioEvent.cpp

@@ -1,9 +1,11 @@
 #include "AudioEvent.hpp"
 
-#include <cstring>
 #include <iostream>
 #include <bits/unique_lock.h>
 
+#include "../../packets/audio/AudioPacketData.hpp"
+#include "../../packets/base/PacketData.hpp"
+
 
 namespace drp::event {
 
@@ -31,13 +33,12 @@ AudioEvent::~AudioEvent() {
 
 void AudioEvent::handle(
     Context& context,
-    const packet::GenericPacketContent& content,
+    const packet::base::PacketContent& content,
     const sockaddr_storage& fromAddress,
     const socklen_t fromAddressLength
 ) {
     // get the audio data in the content
-    packet::AudioPacketData audioData;
-    std::memcpy(&audioData, content.data.data(), sizeof(packet::AudioPacketData));
+    const auto audioData = packet::AudioPacketData::fromGeneric(content);
 
     // save it in the audio queue
     this->audioQueue.push(audioData);

source/events/audio/AudioEvent.hpp

@@ -22,7 +22,7 @@ public:
 
     void handle(
         Context& context,
-        const packet::GenericPacketContent& content,
+        const packet::base::PacketContent& content,
         const sockaddr_storage& fromAddress,
         socklen_t fromAddressLength
     ) override;

source/events/base/BaseEvent.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include "../../packets/base/GenericPacket.hpp"
+#include "../../packets/base/Packet.hpp"
 #include "../../Context.hpp"
 
 
@@ -12,7 +12,7 @@ public:
     virtual ~BaseEvent() = default;
     virtual void handle(
         Context& context,
-        const packet::GenericPacketContent& content,
+        const packet::base::PacketContent& content,
         const sockaddr_storage& fromAddress,
         socklen_t fromAddressLength
     ) = 0;

source/events/info/InfoEvent.cpp

@@ -1,24 +1,25 @@
 #include "InfoEvent.hpp"
 
-#include <cstring>
 #include <iostream>
 #include <sys/socket.h>
 
+#include "../../packets/info/InfoPacketData.hpp"
+
 
 namespace drp::event {
 
 
 void InfoEvent::handle(
     Context& context,
-    const packet::GenericPacketContent& content,
+    const packet::base::PacketContent& content,
     const sockaddr_storage& fromAddress,
     const socklen_t fromAddressLength
 ) {
     std::cout << "Received peer information." << std::endl;
 
     // get the peer information
-    Peer peer;
-    std::memcpy(&peer, content.data.data(), sizeof(Peer));
+    const auto packetData = packet::info::InfoPacketData::fromGeneric(content);
+    const Peer peer = packetData.peer;
 
     // check if the peer address is already in the map
     const auto iterator = context.remotePeers.find(peer.id);

source/events/info/InfoEvent.hpp

@@ -10,7 +10,7 @@ class InfoEvent : public BaseEvent {
 public:
     void handle(
         Context& context,
-        const packet::GenericPacketContent& content,
+        const packet::base::PacketContent& content,
         const sockaddr_storage& fromAddress,
         socklen_t fromAddressLength
     ) override;

source/events/pong/PongEvent.cpp

@@ -8,7 +8,7 @@ namespace drp::event {
 
 void PongEvent::handle(
     Context& context,
-    const packet::GenericPacketContent& content,
+    const packet::base::PacketContent& content,
     const sockaddr_storage& fromAddress,
     const socklen_t fromAddressLength
 ) {

source/events/pong/PongEvent.hpp

@@ -10,7 +10,7 @@ class PongEvent : public BaseEvent {
 public:
     void handle(
         Context& context,
-        const packet::GenericPacketContent& content,
+        const packet::base::PacketContent& content,
         const sockaddr_storage& fromAddress,
         socklen_t fromAddressLength
     ) override;

source/events/search/SearchEvent.cpp

@@ -6,8 +6,8 @@
 #include <iostream>
 #include <ostream>
 
-#include "../../tasks/types.hpp"
-#include "../../events/types.hpp"
+#include "../../packets/base/SecurityMode.hpp"
+#include "../../packets/info/InfoPacketData.hpp"
 
 
 namespace drp {
@@ -15,21 +15,21 @@ namespace drp {
 
 void event::SearchEvent::handle(
     Context& context,
-    const packet::GenericPacketContent& content,
+    const packet::base::PacketContent& content,
     const sockaddr_storage& fromAddress,
     const socklen_t fromAddressLength
 ) {
-    packet::GenericPacket packet {};
-    packet::GenericPacketContent packetContent {};
+    packet::base::Packet packet {};
 
     // create the packet header (available to read for everyone)
     packet.channel = 0;
-    packet.securityMode = static_cast<std::uint8_t>(packet::SecurityMode::PLAIN);
+    packet.securityMode = static_cast<std::uint8_t>(packet::base::SecurityMode::PLAIN);
 
-    // insert our information into the packet
-    packetContent.eventType = static_cast<std::uint8_t>(EventType::INFO);
-    std::memcpy(&packetContent.data, &context.me, sizeof(context.me));
-    packet.setContent(packetContent);
+    // create the packet data containing our information
+    packet::info::InfoPacketData packetData {};
+    packetData.peer = context.me;
+
+    packet.setContent(packetData.toGeneric());
 
     // TODO(Faraphel): send back the timestamp too
 
@@ -46,15 +46,7 @@ void event::SearchEvent::handle(
         return;
     }
 
-    char hoststr[NI_MAXHOST];
-    char portstr[NI_MAXSERV];
-    getnameinfo(
-        reinterpret_cast<const sockaddr*>(&fromAddress), fromAddressLength,
-        hoststr, sizeof(hoststr),
-        portstr, sizeof(portstr),
-        NI_NUMERICHOST | NI_NUMERICSERV
-    );
-    std::cout << "[Receiver] Sent back information to " << hoststr << ":" << portstr << std::endl;
+    std::cout << "[Receiver] Sent back information." << std::endl;
 }
 
 

source/events/search/SearchEvent.hpp

@@ -9,7 +9,7 @@ class SearchEvent : public BaseEvent {
 public:
     void handle(
         Context& context,
-        const packet::GenericPacketContent& content,
+        const packet::base::PacketContent& content,
         const sockaddr_storage& fromAddress,
         socklen_t fromAddressLength
     ) override;

source/packets/audio/AudioPacketData.hpp

@@ -1,14 +1,21 @@
 #pragma once
 
-#include <array>
 #include <chrono>
 #include <cstdint>
 
+#include "../../events/types.hpp"
+#include "../base/PacketData.hpp"
+
 
 namespace drp::packet {
 
 
-struct AudioPacketData {
+/**
+ * Represent the content of an audio packet.
+ * Contains a chunk of audio and its metadata to play it.
+ */
+class AudioPacketData : public base::PacketData<event::EventType::AUDIO, AudioPacketData> {
+public:
     // scheduling
     // TODO(Faraphel): use a more "fixed" size format ?
     std::chrono::time_point<std::chrono::high_resolution_clock> timePlay;
@@ -22,4 +29,4 @@ struct AudioPacketData {
 };
 
 
-}
+}

source/packets/base/GenericPacket.hpp

@@ -1,49 +0,0 @@
-#pragma once
-
-#include <array>
-#include <cstdint>
-
-
-namespace drp::packet {
-
-
-enum class SecurityMode {
-    PLAIN = 0x00,
-    AES = 0x01,
-    RSA = 0x02,
-};
-
-
-// the maximum data length
-// a packet can't be larger than 65565 (uint16 max)
-// reserve some space for metadata and settings
-constexpr std::uint16_t dataLength = 65504;
-
-/**
- * The content of a generic packet.
- * @param eventType the type of event that the packet want to trigger.
- * @param data the data of the event.
- */
-struct GenericPacketContent {
-    std::uint8_t eventType;
-    std::array<std::uint8_t, dataLength> data;
-};
-
-/**
- * A generic packet that can be transmitted through the network.
- * @param channel the channel of the packet. Two system can be created inside a same network by using different
- * channels value. "0" is used for "broadcast" message across networks.
- * @param securityMode the type of security used in the packet.
- * @param _content the content of the packet. It is encrypted accordingly to the securityMode.
- */
-struct GenericPacket {
-    std::uint8_t channel;
-    std::uint8_t securityMode;
-    GenericPacketContent _content;
-
-    [[nodiscard]] GenericPacketContent getContent() const;
-    void setContent(const GenericPacketContent& content);
-};
-
-
-}

source/packets/base/Packet.cpp

@@ -1,9 +1,11 @@
-#include "GenericPacket.hpp"
+#include "Packet.hpp"
 
 #include <stdexcept>
 
+#include "SecurityMode.hpp"
 
-namespace drp::packet {
+
+namespace drp::packet::base {
 
 
 /*
@@ -47,7 +49,7 @@ GenericPacketContent decryptPacketContentAes(const GenericPacket& packet) {
 */
 
 
-GenericPacketContent GenericPacket::getContent() const {
+PacketContent Packet::getContent() const {
     // TODO(Faraphel): implement RSA and AES
     //   additional "context" argument to hold cryptographic keys ?
 
@@ -68,23 +70,4 @@ GenericPacketContent GenericPacket::getContent() const {
 }
 
 
-void GenericPacket::setContent(const GenericPacketContent &content) {
-    // TODO(Faraphel): implement RSA and AES
-    switch (static_cast<SecurityMode>(this->securityMode)) {
-        case SecurityMode::PLAIN:
-            this->_content = content;
-            return;
-
-        case SecurityMode::AES:
-            throw std::runtime_error("Not implemented.");
-
-        case SecurityMode::RSA:
-            throw std::runtime_error("Not implemented.");
-
-        default:
-            throw std::runtime_error("Unsupported security mode.");
-    }
 }
-
-
-}

source/packets/base/Packet.hpp

@@ -0,0 +1,29 @@
+#pragma once
+
+#include <array>
+#include <cstdint>
+
+#include "PacketContent.hpp"
+
+
+namespace drp::packet::base {
+
+
+/**
+ * A generic packet that can be transmitted through the network.
+ * @param channel the channel of the packet. Two system can be created inside a same network by using different
+ * channels value. "0" is used for "broadcast" message across networks.
+ * @param securityMode the type of security used in the packet.
+ * @param _content the content of the packet. It is encrypted accordingly to the securityMode.
+ */
+struct Packet {
+    std::uint8_t channel;
+    std::uint8_t securityMode;
+    PacketContent _content;
+
+    [[nodiscard]] PacketContent getContent() const;
+    void setContent(const PacketContent& content);
+};
+
+
+}

source/packets/base/PacketContent.cpp

@@ -0,0 +1,31 @@
+#include "PacketContent.hpp"
+
+#include <stdexcept>
+
+#include "Packet.hpp"
+#include "SecurityMode.hpp"
+
+
+namespace drp::packet::base {
+
+
+void Packet::setContent(const PacketContent &content) {
+    // TODO(Faraphel): implement RSA and AES
+    switch (static_cast<SecurityMode>(this->securityMode)) {
+        case SecurityMode::PLAIN:
+            this->_content = content;
+        return;
+
+        case SecurityMode::AES:
+            throw std::runtime_error("Not implemented.");
+
+        case SecurityMode::RSA:
+            throw std::runtime_error("Not implemented.");
+
+        default:
+            throw std::runtime_error("Unsupported security mode.");
+    }
+}
+
+
+}

source/packets/base/PacketContent.hpp

@@ -0,0 +1,28 @@
+#pragma once
+
+#include <array>
+#include <cstdint>
+
+
+namespace drp::packet::base {
+
+
+// the maximum data length
+// a packet can't be larger than 65565 (uint16 max)
+// reserve some space for metadata and settings
+constexpr std::uint16_t dataLength = 65504;
+
+
+/**
+ * The content of a generic packet.
+ * @param eventType the type of event that the packet want to trigger.
+ * @param data the data of the event.
+ */
+class PacketContent {
+public:
+    std::uint8_t eventType;
+    std::array<std::uint8_t, dataLength> data;
+};
+
+
+}

source/packets/base/PacketData.hpp

@@ -0,0 +1,48 @@
+#pragma once
+
+#include <cstring>
+
+#include "PacketContent.hpp"
+#include "../../events/types.hpp"
+
+
+namespace drp::packet::base {
+
+
+/**
+ * Represent the actual data contained inside a packet, with no header.
+ * Can be used to implement and communicate anything.
+ * @tparam eventType the event type associated with this type of packet data.
+ * Allow the receiver to redirect this packet to the correct handler.
+ */
+template<event::EventType eventType, class packetClass>
+class PacketData {
+public:
+    /**
+     * Convert this packet data to a generic packet content.
+     * @return a generic packet content.
+     */
+    [[nodiscard]] PacketContent toGeneric() const {
+        // create an empty generic packet content
+        PacketContent content {};
+        // set its content
+        content.eventType = static_cast<std::uint8_t>(eventType);
+        std::memcpy(&content.data, this, sizeof(content));
+
+        return content;
+    }
+
+    /**
+     * Get the data from a generic packet data.
+     * @param content a generic packet content.
+     * @return the actual packet data.
+     */
+    static packetClass fromGeneric(const PacketContent& content) {
+        packetClass data;
+        std::memcpy(&data, content.data.data(), sizeof(packetClass));
+        return data;
+    }
+};
+
+
+}

source/packets/base/SecurityMode.hpp

@@ -0,0 +1,14 @@
+#pragma once
+
+
+namespace drp::packet::base {
+
+
+enum class SecurityMode {
+    PLAIN = 0x00,
+    AES = 0x01,
+    RSA = 0x02,
+};
+
+
+}

source/packets/info/InfoPacketData.hpp

@@ -0,0 +1,21 @@
+#pragma once
+
+#include "../../RemotePeer.hpp"
+#include "../../events/types.hpp"
+#include "../base/PacketData.hpp"
+
+
+namespace drp::packet::info {
+
+
+/**
+ * Represent the content of an info packet.
+ * Contains information about the peer sending it.
+ */
+class InfoPacketData : public base::PacketData<event::EventType::INFO, InfoPacketData> {
+public:
+    Peer peer {};
+};
+
+
+}

source/packets/search/SearchPacketData.hpp

@@ -1,13 +1,21 @@
 #pragma once
+
 #include <chrono>
 
-
-namespace drp::packet {
+#include "../base/PacketData.hpp"
 
 
-typedef struct {
+namespace drp::packet::search {
+
+
+/**
+ * Represent a discovery request.
+ * Sent by someone to get information about other available machine in the network.
+ */
+class SearchPacketData : public base::PacketData<event::EventType::SEARCH, SearchPacketData> {
+public:
     std::chrono::time_point<std::chrono::system_clock> timestamp;  /// timestamp when the search request was sent
-} SearchPacketData;
+};
 
 
 }

source/tasks/client/ClientTask.cpp

@@ -1,5 +1,6 @@
 #include "ClientTask.hpp"
 
+#include <iostream>
 #include <thread>
 
 
@@ -7,7 +8,21 @@ namespace drp::task {
 
 
 void ClientTask::handle(Context& context) {
-    // TODO(Faraphel): connect to an ntp server
+    // get the server hostname
+    char host[NI_MAXHOST];
+    char port[NI_MAXSERV];
+    getnameinfo(
+        reinterpret_cast<sockaddr*>(&context.server->address), context.server->addressLength,
+        host, sizeof(host),
+        port, sizeof(port),
+        NI_NUMERICHOST | NI_NUMERICSERV
+    );
+
+    // connect to the chrony server
+    // TODO(Faraphel): only once ?
+    FILE* chronyProcess = popen(("chronyc add server " + std::string(host) + " 2>&1").c_str(), "r");
+    if (pclose(chronyProcess) == -1)
+        std::cerr << "Failed to connect to chrony server !" << std::endl;
 
     // TODO(Faraphel): check if the server is still reachable.
     //   if connection lost, go back to undefined mode.

source/tasks/server/ServerTask.cpp

@@ -10,9 +10,9 @@
 #include <thread>
 #include <sys/socket.h>
 
-#include "../../events/types.hpp"
 #include "../../packets/audio/AudioPacketData.hpp"
-#include "../../packets/base/GenericPacket.hpp"
+#include "../../packets/base/Packet.hpp"
+#include "../../packets/base/SecurityMode.hpp"
 #include "../../utils/audio.hpp"
 
 
@@ -57,17 +57,22 @@ ServerTask::~ServerTask() {
 void ServerTask::handle(Context& context) {
     // TODO(Faraphel): create a chrony server
 
-    // read the file
-    packet::GenericPacket packet {};
-    packet::GenericPacketContent packetContent {};
+    // allow anybody to connect as a chrony client
+    FILE* chronyProcess = popen("chronyc allow all 2>&1", "r");
+    if (pclose(chronyProcess) == -1)
+        std::cerr << "Could not allow chrony connection." << std::endl;
+
+    // prepare the packet structure
+    packet::base::Packet packet {};
     packet::AudioPacketData audioPacket;
     std::size_t done;
 
     // create a packet
     // TODO(Faraphel): should not be broadcast plaintext
     packet.channel = 0;
-    packet.securityMode = static_cast<std::uint8_t>(packet::SecurityMode::PLAIN);
+    packet.securityMode = static_cast<std::uint8_t>(packet::base::SecurityMode::PLAIN);
 
+    // read the file
     if (mpg123_read(
         this->mpgHandle,
         &audioPacket.content,
@@ -93,10 +98,7 @@ void ServerTask::handle(Context& context) {
     audioPacket.sampleFormat = util::encoding_mpg123_to_PulseAudio(this->encoding);
     audioPacket.sampleRate = this->sampleRate;
 
-    // wrap the audio packet in the content
-    packetContent.eventType = static_cast<std::uint8_t>(event::EventType::AUDIO);
-    std::memcpy(packetContent.data.data(), &audioPacket, packetContent.data.size());
-    packet.setContent(packetContent);
+    packet.setContent(audioPacket.toGeneric());
 
     // broadcast the audio data
     if (sendto(

source/tasks/undefined/UndefinedTask.cpp

@@ -10,8 +10,8 @@
 #include <sys/socket.h>
 
 #include "../../Context.hpp"
-#include "../../events/types.hpp"
-#include "../../packets/base/GenericPacket.hpp"
+#include "../../packets/base/Packet.hpp"
+#include "../../packets/base/SecurityMode.hpp"
 #include "../../packets/search/SearchPacketData.hpp"
 
 
@@ -75,18 +75,17 @@ void UndefinedTask::handle(Context& context) {
     }
 
     // prepare a search message
-    packet::GenericPacket packet {};
-    packet::GenericPacketContent packetContent {};
-    packet::SearchPacketData packetData {};
+    packet::base::Packet packet {};
+    packet::search::SearchPacketData packetData {};
 
     // broadcast message
     packet.channel = 0;
-    packet.securityMode = static_cast<std::uint8_t>(packet::SecurityMode::PLAIN);
+    packet.securityMode = static_cast<std::uint8_t>(packet::base::SecurityMode::PLAIN);
+
     // search message with the time of the message being sent
-    packetContent.eventType = static_cast<std::uint8_t>(event::EventType::SEARCH);
     packetData.timestamp = std::chrono::high_resolution_clock::now();
-    std::memcpy(&packetContent.data, &packetData, sizeof(packetData));
-    packet.setContent(packetContent);
+
+    packet.setContent(packetData.toGeneric());
 
     std::cout << "Looking for new peers." << std::endl;