include review comments, will be squashed

Author: sudden6

auto_tests/toxav_mt_test.cc

@@ -38,15 +38,20 @@ using ToxAV_Ptr = std::unique_ptr<ToxAV, ToxAV_Deleter>;
 static void t_accept_friend_request_cb(Tox *m, const uint8_t *public_key, const uint8_t *data,
                                        size_t length, void *userdata) {
   if (length == 7 && std::memcmp("gentoo", data, 7) == 0) {
-    ck_assert(tox_friend_add_norequest(m, public_key, nullptr) != (uint32_t)-1);
+    Tox_Err_Friend_Add err;
+    tox_friend_add_norequest(m, public_key, &err);
+    ck_assert(err == TOX_ERR_FRIEND_ADD_OK);
+  } else {
+    // No other request expected
+    ck_assert(false);
   }
 }
 
-std::vector<Tox_Ptr> prepareNetwork(uint32_t count) {
+std::vector<Tox_Ptr> prepare_network(uint32_t count) {
   Tox_Err_New error;
 
   // Temporary bootstrap node
-  std::printf("Created 1 instance of Tox as boostrap node\n");
+  std::printf("Created 1 instance of Tox as bootstrap node\n");
   Tox_Ptr bootstrap = Tox_Ptr(tox_new_log(nullptr, &error, nullptr));
   ck_assert(error == TOX_ERR_NEW_OK);
 
@@ -81,12 +86,12 @@ std::vector<Tox_Ptr> prepareNetwork(uint32_t count) {
   // temporarily add bootstrap node to end of toxes, so we can iterate all
   toxes.push_back(std::move(bootstrap));
 
-  uint32_t bootstrap_iteration = 0;
   bool online = false;
 
   auto bootstrap_start_time = Clock::now();
 
-  for (; bootstrap_iteration < MAX_BOOTSTRAP_ITERATIONS; ++bootstrap_iteration) {
+  uint32_t bootstrap_iteration;
+  for (bootstrap_iteration = 0; bootstrap_iteration < MAX_BOOTSTRAP_ITERATIONS; ++bootstrap_iteration) {
     for (auto &tox : toxes) {
       tox_iterate(tox.get(), nullptr);
     }
@@ -140,12 +145,12 @@ class AV_State {
  public:
   explicit AV_State(Tox_Ptr tox, std::string name, bool combined = false)
       : tox_(std::move(tox)),
-        combined_(combined),
-        stop_threads{false},
-        incomming{false},
-        call_state{0},
-        video_received{false},
-        audio_received{false},
+        combined_av_(combined),
+        stop_threads_{false},
+        incomming_{false},
+        call_state_{0},
+        video_received_{false},
+        audio_received_{false},
         name_{name} {
     Toxav_Err_New error;
     toxAV_ = ToxAV_Ptr(toxav_new(tox_.get(), &error));
@@ -156,40 +161,40 @@ class AV_State {
     toxav_callback_video_receive_frame(toxAV_.get(), &AV_State::toxav_receive_video_frame_cb, this);
     toxav_callback_audio_receive_frame(toxAV_.get(), &AV_State::toxav_receive_audio_frame_cb, this);
 
-    tox_thread = std::thread(&AV_State::tox_iterator, this);
+    tox_thread_ = std::thread(&AV_State::tox_iterator, this);
 
     if (combined) {
-      av_thread = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_BOTH);
+      av_thread_ = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_BOTH);
     } else {
-      audio_thread = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_AUDIO);
-      video_thread = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_VIDEO);
+      audio_thread_ = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_AUDIO);
+      video_thread_ = std::thread(&AV_State::toxav_iterator, this, Iteration_Type::TOXAV_VIDEO);
     }
   }
 
-  ~AV_State() { stopThreads(); }
+  ~AV_State() { stop_threads(); }
 
-  ToxAV *getToxAV() const { return toxAV_.get(); }
-  std::mutex &getToxLoopLock() { return tox_loop_lock; }
-  bool inCall() const { return in_call.load(); }
-  uint32_t getCallState() const { return call_state.load(); }
-  void stopThreads() {
-    if (stop_threads.exchange(true)) {
+  ToxAV *get_ToxAV() const { return toxAV_.get(); }
+  std::mutex &get_tox_loop_lock() { return tox_loop_lock_; }
+  bool in_call() const { return in_call_.load(); }
+  uint32_t get_call_state() const { return call_state_.load(); }
+  void stop_threads() {
+    if (stop_threads_.exchange(true)) {
       // already stopped
       return;
     }
 
-    tox_thread.join();
+    tox_thread_.join();
 
-    if (combined_) {
-      av_thread.join();
+    if (combined_av_) {
+      av_thread_.join();
     } else {
-      audio_thread.join();
-      video_thread.join();
+      audio_thread_.join();
+      video_thread_.join();
     }
   }
 
-  bool didReceiveAudio() const { return audio_received.load(); }
-  bool didReceiveVideo() const { return video_received.load(); }
+  bool did_receive_audio() const { return audio_received_.load(); }
+  bool did_receive_video() const { return video_received_.load(); }
 
   static constexpr uint32_t TEST_A_BITRATE = 48;    // In kbit/s
   static constexpr uint32_t TEST_V_BITRATE = 4000;  // In kbit/s
@@ -206,22 +211,22 @@ class AV_State {
                             bool video_enabled, void *user_data) {
     AV_State *me = static_cast<AV_State *>(user_data);
     std::cout << "[" << me->name_ << "] Handling CALL callback" << std::endl;
-    me->incomming.store(true);
-    ck_assert(std::this_thread::get_id() == me->tox_thread.get_id());
+    me->incomming_.store(true);
+    ck_assert(std::this_thread::get_id() == me->tox_thread_.get_id());
   }
 
   static void toxav_call_state_cb(ToxAV *av, uint32_t friend_number, uint32_t state,
                                   void *user_data) {
     AV_State *me = static_cast<AV_State *>(user_data);
-    ck_assert(std::this_thread::get_id() == me->tox_thread.get_id());
+    ck_assert(std::this_thread::get_id() == me->tox_thread_.get_id());
     ck_assert(state != TOXAV_FRIEND_CALL_STATE_ERROR);
     std::cout << "[" << me->name_ << "] State changed to: " << std::to_string(state) << std::endl;
-    me->call_state.store(state);
-    TimePoint tp = me->call_start.load();
+    me->call_state_.store(state);
+    TimePoint tp = me->call_start_.load();
 
     if (state != TOXAV_FRIEND_CALL_STATE_NONE && tp == TimePoint()) {
-      me->call_start.store(Clock::now());
-      me->in_call.store(true);
+      me->call_start_.store(Clock::now());
+      me->in_call_.store(true);
     }
   }
 
@@ -233,13 +238,13 @@ class AV_State {
     std::cout << "[" << me->name_ << "] Received video payload" << std::endl;
 
     // toxav.h states that receive events are emitted from their respective threads
-    if (me->combined_) {
-      ck_assert(std::this_thread::get_id() == me->av_thread.get_id());
+    if (me->combined_av_) {
+      ck_assert(std::this_thread::get_id() == me->av_thread_.get_id());
     } else {
-      ck_assert(std::this_thread::get_id() == me->video_thread.get_id());
+      ck_assert(std::this_thread::get_id() == me->video_thread_.get_id());
     }
 
-    me->video_received = true;
+    me->video_received_ = true;
   }
 
   static void toxav_receive_audio_frame_cb(ToxAV *av, uint32_t friend_number, int16_t const *pcm,
@@ -248,24 +253,24 @@ class AV_State {
     AV_State *me = static_cast<AV_State *>(user_data);
     std::cout << "[" << me->name_ << "] Received audio payload" << std::endl;
 
-    if (me->combined_) {
-      ck_assert(std::this_thread::get_id() == me->av_thread.get_id());
+    if (me->combined_av_) {
+      ck_assert(std::this_thread::get_id() == me->av_thread_.get_id());
     } else {
-      ck_assert(std::this_thread::get_id() == me->audio_thread.get_id());
+      ck_assert(std::this_thread::get_id() == me->audio_thread_.get_id());
     }
 
-    me->audio_received = true;
+    me->audio_received_ = true;
   }
 
   void tox_iterator() {
-    while (!stop_threads.load()) {
+    while (!stop_threads_.load()) {
       // Perform this block only while loop lock is held
       {
-        std::lock_guard<std::mutex> lock(tox_loop_lock);
+        std::lock_guard<std::mutex> lock(tox_loop_lock_);
         tox_iterate(tox_.get(), this);
 
         // handle incoming call
-        if (incomming.exchange(false)) {
+        if (incomming_.exchange(false)) {
           Toxav_Err_Answer answer_err;
           toxav_answer(toxAV_.get(), 0, TEST_A_BITRATE, TEST_V_BITRATE, &answer_err);
 
@@ -276,19 +281,19 @@ class AV_State {
 
           std::cout << "[" << name_ << "] Answering call" << std::endl;
 
-          call_start = Clock::now();
-          in_call.store(true);
+          call_start_ = Clock::now();
+          in_call_.store(true);
         }
 
-        if (in_call.load()) {
-          uint32_t state = call_state.load();
-          TimePoint tp = call_start.load();
+        if (in_call_.load()) {
+          uint32_t state = call_state_.load();
+          TimePoint tp = call_start_.load();
           std::chrono::duration<double> call_time = Clock::now() - tp;
 
           if (state == TOXAV_FRIEND_CALL_STATE_FINISHED) {
             std::cout << "[" << name_ << "] Call ended by other side after: " << call_time.count()
                       << "s" << std::endl;
-            in_call.store(false);
+            in_call_.store(false);
           } else if (tp > TimePoint() && call_time > AV_State::AUTO_HANGUP_TIME) {
             std::cout << "[" << name_ << "] Ending call after: " << call_time.count() << "s"
                       << std::endl;
@@ -302,7 +307,7 @@ class AV_State {
               ck_assert(0);
             }
 
-            in_call.store(false);
+            in_call_.store(false);
           }
         }
       }
@@ -312,7 +317,7 @@ class AV_State {
   }
 
   void toxav_iterator(Iteration_Type type) {
-    while (!stop_threads.load()) {
+    while (!stop_threads_.load()) {
       switch (type) {
         case Iteration_Type::TOXAV_AUDIO:
           toxav_audio_iterate(toxAV_.get());
@@ -335,26 +340,26 @@ class AV_State {
     }
   }
 
-  std::thread tox_thread;
-  std::thread audio_thread;
-  std::thread video_thread;
-  std::thread av_thread;
+  std::thread tox_thread_;
+  std::thread audio_thread_;
+  std::thread video_thread_;
+  std::thread av_thread_;
 
-  std::mutex tox_loop_lock;
+  std::mutex tox_loop_lock_;
 
   Tox_Ptr tox_;
-  bool combined_;
+  bool combined_av_;
   ToxAV_Ptr toxAV_;
 
-  std::atomic_bool stop_threads;
-  std::atomic_bool incomming;
-  std::atomic_uint32_t call_state;
+  std::atomic_bool stop_threads_;
+  std::atomic_bool incomming_;
+  std::atomic_uint32_t call_state_;
 
-  std::atomic<TimePoint> call_start;
-  std::atomic_bool in_call;
+  std::atomic<TimePoint> call_start_;
+  std::atomic_bool in_call_;
 
-  std::atomic_bool video_received;
-  std::atomic_bool audio_received;
+  std::atomic_bool video_received_;
+  std::atomic_bool audio_received_;
   std::string name_;
 };
 
@@ -382,33 +387,29 @@ constexpr uint8_t DUMMY_VIDEO::y[];
 constexpr uint8_t DUMMY_VIDEO::u[];
 constexpr uint8_t DUMMY_VIDEO::v[];
 
-}  // namespace
-
-static void test_av(bool combined) {
-  std::cout << "Testing Audio and Video in" << (combined ? "combined" : "separate") << " threads"
+static void test_av(bool combined_av) {
+  std::cout << "Testing Audio and Video in" << (combined_av ? "combined" : "separate") << " threads"
             << std::endl;
-  auto toxes = prepareNetwork(2);
+  auto toxes = prepare_network(2);
 
   AV_State alice(std::move(toxes[0]), "alice", false);
   AV_State bob(std::move(toxes[1]), "bob", false);
 
   // Let alice call bob
   {
-    std::lock_guard<std::mutex>(alice.getToxLoopLock());
+    std::lock_guard<std::mutex>(alice.get_tox_loop_lock());
     Toxav_Err_Call err;
     ck_assert(
-        toxav_call(alice.getToxAV(), 0, AV_State::TEST_A_BITRATE, AV_State::TEST_V_BITRATE, &err));
+        toxav_call(alice.get_ToxAV(), 0, AV_State::TEST_A_BITRATE, AV_State::TEST_V_BITRATE, &err));
     ck_assert(err == TOXAV_ERR_CALL_OK);
   }
 
   std::cout << "alice started a call" << std::endl;
 
   auto poll_state = [](AV_State &av, uint32_t expected, uint32_t max_tries,
                        uint32_t delay_ms) -> bool {
-    uint32_t i;
-
-    for (i = 0; i < max_tries; ++i) {
-      uint32_t state = av.getCallState();
+    for (uint32_t i = 0; i < max_tries; ++i) {
+      uint32_t state = av.get_call_state();
 
       if (state == expected) {
         return true;
@@ -431,8 +432,8 @@ static void test_av(bool combined) {
   // TODO: why is Bob's call state not updated?
   // ck_assert(poll_state(bob, full_AV_mask, 10, 100)); asserts
 
-  ck_assert(alice.inCall());
-  ck_assert(bob.inCall());
+  ck_assert(alice.in_call());
+  ck_assert(bob.in_call());
 
   std::cout << "alice and bob are in the call" << std::endl;
 
@@ -448,23 +449,23 @@ static void test_av(bool combined) {
 
   // Send frames from alice to bob
   {
-    std::lock_guard<std::mutex>(alice.getToxLoopLock());
+    std::lock_guard<std::mutex>(alice.get_tox_loop_lock());
     Toxav_Err_Send_Frame err;
-    ck_assert(toxav_audio_send_frame_dummy(alice.getToxAV(), &err));
+    ck_assert(toxav_audio_send_frame_dummy(alice.get_ToxAV(), &err));
     ck_assert(err == TOXAV_ERR_SEND_FRAME_OK);
 
-    ck_assert(toxav_video_send_frame_dummy(alice.getToxAV(), &err));
+    ck_assert(toxav_video_send_frame_dummy(alice.get_ToxAV(), &err));
     ck_assert(err == TOXAV_ERR_SEND_FRAME_OK);
   }
 
   // Send frames from bob to alice
   {
-    std::lock_guard<std::mutex>(bob.getToxLoopLock());
+    std::lock_guard<std::mutex>(bob.get_tox_loop_lock());
     Toxav_Err_Send_Frame err;
-    ck_assert(toxav_audio_send_frame_dummy(bob.getToxAV(), &err));
+    ck_assert(toxav_audio_send_frame_dummy(bob.get_ToxAV(), &err));
     ck_assert(err == TOXAV_ERR_SEND_FRAME_OK);
 
-    ck_assert(toxav_video_send_frame_dummy(bob.getToxAV(), &err));
+    ck_assert(toxav_video_send_frame_dummy(bob.get_ToxAV(), &err));
     ck_assert(err == TOXAV_ERR_SEND_FRAME_OK);
   }
 
@@ -474,20 +475,22 @@ static void test_av(bool combined) {
   // TODO: why is Bobs call state not updated?
   // ck_assert(poll_state(bob, TOXAV_FRIEND_CALL_STATE_FINISHED, 30, 100)); fails
 
-  ck_assert(!alice.inCall());
-  ck_assert(!bob.inCall());
+  ck_assert(!alice.in_call());
+  ck_assert(!bob.in_call());
 
   std::cout << "The call ended" << std::endl;
 
-  alice.stopThreads();
-  bob.stopThreads();
+  alice.stop_threads();
+  bob.stop_threads();
 
-  ck_assert(alice.didReceiveAudio());
-  ck_assert(alice.didReceiveVideo());
-  ck_assert(bob.didReceiveAudio());
-  ck_assert(bob.didReceiveVideo());
+  ck_assert(alice.did_receive_audio());
+  ck_assert(alice.did_receive_video());
+  ck_assert(bob.did_receive_audio());
+  ck_assert(bob.did_receive_video());
 }
 
+}  // namespace
+
 int main(void) {
   setvbuf(stdout, nullptr, _IONBF, 0);