[Indy]IndyTextEncoding

[Lena]

Hi.
I try send lines from Memo to Memo on Server.
My code Server:

Code: Select all

//h
#include <IdSync.hpp>
class TUpdateRichEdit : public TIdNotify
{
   protected:
      String sText;
      void __fastcall DoNotify();
   public:
      __fastcall TUpdateRichEdit( const String &sMessage );
};

//***********************
//cpp:

TFormPrint *FormPrint;
//---------------------------------------------------------------------------
__fastcall TFormPrint::TFormPrint(TComponent* Owner)
   : TForm(Owner)
{
 IdTCPServer1->Active = true;
}
//---------------------------------------------------------------------------

__fastcall TUpdateRichEdit::TUpdateRichEdit( const String &sMessage )
   : TIdNotify(), sText(sMessage)
{
}
//---------------------------------------------------------------------------

void __fastcall TUpdateRichEdit::DoNotify()
{
 if (sText.IsEmpty()) return;


 FormPrint->Memo1->Lines->Clear();
 FormPrint->Memo1->Text = sText;
}
//---------------------------------------------------------------------------

void ZapisVRichEditWindow(String zapis)
{

   TUpdateRichEdit *updateLB = new TUpdateRichEdit(zapis);
   try
   {
      updateLB->Notify();
   }
   catch (const Exception &)
     {
      delete updateLB;
      throw;
     }

}
//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
 try
    {
     String Sdata = AContext->Connection->IOHandler->AllData(IndyTextEncoding_UTF8());
     if (Sdata.IsEmpty()) return;

     ZapisVRichEditWindow(Sdata);

     }
    catch (const Exception &E)
      {
        String mis = L"Error class: " +   E.ClassName() + L" Error message: " +  E.Message;
        ZapisVRichEditWindow(mis);
      }
}


Code fo client:

Code: Select all

void __fastcall TFormSent::Button1Click(TObject *Sender)
{
  try
  {

   IdTCPClient1->Connect();
   IdTCPClient1->IOHandler->WriteLn(Memo1->Lines->Text);

  }
   __finally
  {
   IdTCPClient1->Disconnect();
  }
}


Two questions:
1. Your recommendation for correct try and catch in my code.
2. How to get Russian letters on the server?
In Memo1 client lines:

Code: Select all

Строка ... 1
Строка ... 2
Строка ... 3
__________
Итого ... 3


In server I got in Memo lines:

Code: Select all

?????? ... 1
?????? ... 2
?????? ... 3
__________
????? ... 3


[rlebeau]

Code: Select all

void __fastcall TUpdateRichEdit::DoNotify()
{
 if (sText.IsEmpty()) return;


 FormPrint->Memo1->Lines->Clear();
 FormPrint->Memo1->Text = sText;
}


You do not need to Clear() the Memo before assigning new Text to it. And why would you ignore a blank string if that is what was actually transmitted? Besides, you are not passing a blank string to ZapisVRichEditWindow() anyway, so TUpdateRichEdit doesn't need to check for a blank string.

Code: Select all

void __fastcall TUpdateRichEdit::DoNotify()
{
    FormPrint->Memo1->Text = sText;
}


Code: Select all

String Sdata = AContext->Connection->IOHandler->AllData(IndyTextEncoding_UTF8());


AllData() does not exit until the client disconnects. Is that what you really want? For that matter, your client is using IOHandler->WriteLn() when it could just use IOHandler->Write() instead.

Unless your clients send data very infrequently, requiring clients to reconnect just to send 1 string is not a very good idea. At the very least, don't disconnect a client unless it has been idle for awhile, say 5-10 minutes. That way, clients that send data frequently benefit from reusing a persistent connection. A simple way to handle that on the server side is to set the IOHandler's ReadTimeout method, and let Indy throw an exception that TIdTCPServer handles if a read operation times out.

Also, instead of using IOHandler->WriteLn() and IOHandler->AllData(), I would suggest using either:

• IOHandler->Write(TStrings) and IOHandler->ReadStrings():
  
  

  Code: Select all

   IdTCPClient1->IOHandler->Write(Memo1->Lines, true);
 

  
  
  

  Code: Select all

   TStringList *Sdata = new TStringList;
 AContext->Connection->IOHandler->ReadStrings(strs);
 ...
 delete Sdata;
 

  
  
• IOHandler->Write() and IOHandler->ReadString(), preceding the string with its byte length:
  
  

  Code: Select all

   TIdBytes bytes = IndyTextEncoding_UTF8()->GetBytes(Memo1->Lines->Text);
 IdTCPClient1->IOHandler->Write(Int32(bytes.Length));
 IdTCPClient1->IOHandler->Write(bytes);
 

  
  
  

  Code: Select all

   int len = AContext->Connection->IOHandler->ReadInt32();
 String Sdata = AContext->Connection->IOHandler->ReadString(len);
 

Code: Select all

catch (const Exception &E)
{
    String mis = L"Error class: " +   E.ClassName() + L" Error message: " +  E.Message;
    ZapisVRichEditWindow(mis);
}


Your OnExecute handler is catching ALL exceptions and discarding them. DO NOT do that. TIdTCPServer relies on being able to handle exceptions for its connection management. At the very least, you should re-throw any EIdException-derived exception you catch:

Code: Select all

catch (const Exception &E)
{
    ...
    if (dynamic_cast<const EIdException*>(&E) != NULL)
        throw;
}


Otherwise, get rid of your try/catch in OnExecute and use the OnException event instead:

Code: Select all

void __fastcall TFormPrint::IdTCPServer1Connect(TIdContext *AContext)
{
    AContext->Connection->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();
}

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
    String Sdata = AContext->Connection->IOHandler->AllData();
    if (!Sdata.IsEmpty()) ZapisVRichEditWindow(Sdata);
}

void __fastcall TFormPrint::IdTCPServer1Exception(TIdContext *AContext, Exception *AException)
{
    if (!dynamic_cast<EIdConnClosedGracefully*>(AException))
    {
        String mis = _D("Error class: ") + AException->ClassName() + _D(" Error message: ") + AException->Message;
        ZapisVRichEditWindow(mis);
    }
}


Code: Select all

IdTCPClient1->IOHandler->WriteLn(Memo1->Lines->Text);


Your server is expecting clients to send text in UTF-8, but your client code is not specifying any encoding at all, so it will use Indy's default of ASCII instead (see the global GIdDefaultTextEncoding variable in IdGlobal.hpp). That is why you are losing your Russian characters.

You need to specify UTF-8 when sending strings to the server, either by:

• passing IndyTextEncoding_UTF8 directly to WriteLn() (or Write(TStrings)):
  
  

  Code: Select all

   IdTCPClient1->IOHandler->WriteLn(Memo1->Lines->Text, IndyTextEncoding_UTF8());
 

  
  
  

  Code: Select all

   IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
 

  
  
• setting IdTCPClient1->IOHandler->DefStringEncoding to IndyTextEncoding_UTF8 after calling Connect() and before calling WriteLn() (or Write(TStrings)):
  
  

  Code: Select all

   IdTCPClient1->Connect();
 IdTCPClient1->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();
 IdTCPClient1->IOHandler->WriteLn(Memo1->Lines->Text);
 //IdTCPClient1->IOHandler->Write(Memo1->Lines, true);
 

  
  
• setting GIdDefaultTextEncoding to encUTF8, such as at program startup:
  
  

  Code: Select all

   GIdDefaultTextEncoding  = encUTF8;
 ...
 IdTCPClient1->IOHandler->WriteLn(Memo1->Lines->Text);
 //IdTCPClient1->IOHandler->Write(Memo1->Lines, true);
 

[Lena]

Thank You very much! Now I see Russian letters.
My first attempt and all is well:
Server:

Code: Select all

//h
class TUpdateRichEdit : public TIdNotify
{
   protected:
      String sText;
      void __fastcall DoNotify();
   public:
      __fastcall TUpdateRichEdit( const String &sMessage );
};


//cpp
#include <vcl.h>
#include <Vcl.Printers.hpp>
#include <memory>
#pragma hdrstop

#include "UnitPrint.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TFormPrint *FormPrint;
//---------------------------------------------------------------------------
__fastcall TFormPrint::TFormPrint(TComponent* Owner)
   : TForm(Owner)
{
 IdTCPServer1->Active = true;
}
//---------------------------------------------------------------------------

__fastcall TUpdateRichEdit::TUpdateRichEdit( const String &sMessage )
   : TIdNotify(), sText(sMessage)
{
}
//---------------------------------------------------------------------------

void __fastcall TUpdateRichEdit::DoNotify()
{
 FormPrint->Memo1->Lines->Clear();
 FormPrint->Memo1->Text = sText;

 if(FormPrint->Memo1->Lines->Count <= 0)
      return;

   TPrinter *Prntr = Printer();
   TRect r = Rect(200, 200, Prntr->PageWidth - 200, Prntr->PageHeight - 200);
   Prntr->BeginDoc();
   for (int i = 0; i < FormPrint->Memo1->Lines->Count; i++)
      Prntr->Canvas->TextOut(200,
         200 + (i * Prntr->Canvas->TextHeight(FormPrint->Memo1->Lines->Strings[i])),
         FormPrint->Memo1->Lines->Strings[i]);
   Prntr->Canvas->Brush->Color = clBlack;
   //Prntr->Canvas->FrameRect(r);
   Prntr->EndDoc();

}
//---------------------------------------------------------------------------

void ZapisVRichEditWindow(String zapis)
{

   TUpdateRichEdit *updateLB = new TUpdateRichEdit(zapis);
   try
   {
    updateLB->Notify();
   }
   catch (const Exception &)
     {
      delete updateLB;
      throw;
     }

}
//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
 try
    {
     std::unique_ptr<TStringList> Sdata(new TStringList);
     AContext->Connection->IOHandler->ReadStrings(Sdata.get());

     if (Sdata->Text.IsEmpty())
            return;

     ZapisVRichEditWindow(Sdata->Text);

    }
    catch (const Exception &E)
      {
        String mis = L"Error class: " +
                     E.ClassName() + L" Error Message: " +  E.Message;

        ZapisVRichEditWindow(mis);
          if (dynamic_cast<const EIdException*>(&E) != NULL)
                 throw;
      }
}
//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Connect(TIdContext *AContext)
{
 //Very important for Russian letters
 AContext->Connection->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();
}


Client:

Code: Select all

void __fastcall TFormSent::Button1Click(TObject *Sender)
{
  try
  {
   try{
      IdTCPClient1->Connect();
      IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
     }
     catch(const Exception &E)
         {
         String mis = L"Error class: " +   E.ClassName() + L" Error message: " +  E.Message;
            ShowMessage(mis);

            if (dynamic_cast<const EIdException*>(&E) != NULL)
            throw;
           }

  }
   __finally
  {
   IdTCPClient1->Disconnect();
  }
}


And why would you ignore a blank string if that is what was actually transmitted?

Because after receiving the strings, I start printing to the printer...
I checked the transmission russian strings and printing, it seems that everything works as it should now.

[Lena]

No, there are problems.
The printer prints normally only the first time. All subsequent attempts print black lines filled with ink.

P.S.
ShowMessage=Error class: EIdConnClosedGracefully Error Message: Connection Closed Gracefully.

[Lena]

I made small changes and now the printer has printed five times everything is normal...

Code: Select all

#include <vcl.h>
#include <Vcl.Printers.hpp>
#include <memory>
#pragma hdrstop

#include "UnitPrint.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TFormPrint *FormPrint;
//---------------------------------------------------------------------------
__fastcall TFormPrint::TFormPrint(TComponent* Owner)
   : TForm(Owner)
{
 IdTCPServer1->Active = true;
}
//---------------------------------------------------------------------------

__fastcall TUpdateRichEdit::TUpdateRichEdit( const String &sMessage )
   : TIdNotify(), sText(sMessage)
{
}
//---------------------------------------------------------------------------

void __fastcall TUpdateRichEdit::DoNotify()
{
 FormPrint->Memo1->Lines->Clear();
 FormPrint->Memo1->Text = sText;

 if(FormPrint->Memo1->Lines->Count <= 0)
      return;

     std::unique_ptr<TPrinter> Prntr(new TPrinter); //like new
     Prntr->BeginDoc();
     for (int i = 0; i < FormPrint->Memo1->Lines->Count; i++)
      Prntr->Canvas->TextOut(200,
         200 + (i * Prntr->Canvas->TextHeight(FormPrint->Memo1->Lines->Strings[i])),
         FormPrint->Memo1->Lines->Strings[i]);
      Prntr->Canvas->Brush->Color = clBlack;
      Prntr->EndDoc();

}
//---------------------------------------------------------------------------

void ZapisVRichEditWindow(String zapis)
{

   TUpdateRichEdit *updateLB = new TUpdateRichEdit(zapis);
   try
   {
    updateLB->Notify();
   }
   catch (const Exception &)
     {
      delete updateLB;
      throw;
     }

}
//---------------------------------------------------------------------------


void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
 try
    {
     std::unique_ptr<TStringList> Sdata(new TStringList);
     AContext->Connection->IOHandler->ReadStrings(Sdata.get());

     if (Sdata->Text.IsEmpty())
            return;

     ZapisVRichEditWindow(Sdata->Text);
     Sdata->Clear();

     //remove ShowMessage=Connection Closed Gracefully
      AContext->Connection->IOHandler->Close();

    }
    catch (const Exception &E)
      {
      String mis = L"Error class: " +   E.ClassName() + L" Error message: " +  E.Message;
         ShowMessage(mis);

        if (dynamic_cast<const EIdException*>(&E) != NULL)
         throw;
      }
}
//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Connect(TIdContext *AContext)
{
 //for russian
 AContext->Connection->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();
}
//---------------------------------------------------------------------------


[rlebeau]

No, there are problems.
ShowMessage=Error class: EIdConnClosedGracefully Error Message: Connection Closed Gracefully.

That is a perfectly normal exception to get, especially on the server side. You are just not ignoring it. And calling IOHandler->Close() does not prevent that exception from being thrown earlier if the client disconnect prematurely before the server is finished reading (besides, you shouldn't be calling IOHandler->Close() directly anyway, use AContext->Connection->Disconnect() instead).

Also, ShowMessage() is not thread-safe, so DO NOT be using it outside of the main UI thread. Either synchronize it, or use the Win32 API MessageBox() instead.

I made small changes and now the printer has printed five times everything is normal...

I would suggest the following further tweaks:

Code: Select all

class TUpdateRichEdit : public TIdNotify
{
private:
   std::unique_ptr<TStringList> sText;
protected:
   virtual void __fastcall DoNotify();
public:
   __fastcall TUpdateRichEdit(std::unique_ptr<TStringList> &sMessage);
};


Code: Select all

#include <vcl.h>
#pragma hdrstop

#include <Vcl.Printers.hpp>
#include <memory>

#include "UnitPrint.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
#pragma resource "*.dfm"
TFormPrint *FormPrint;
//---------------------------------------------------------------------------
__fastcall TFormPrint::TFormPrint(TComponent* Owner)
   : TForm(Owner)
{
   IdTCPServer1->Active = true;
}
//---------------------------------------------------------------------------

__fastcall TUpdateRichEdit::TUpdateRichEdit(std::unique_ptr<TStringList> &sMessage)
   : TIdNotify(), sText(std::move(sMessage))
{
}
//---------------------------------------------------------------------------

void __fastcall TUpdateRichEdit::DoNotify()
{
   FormPrint->Memo1->Lines->Assign(sText.get());

   int count = sText->Count;
   if (count == 0)
      return;

   std::unique_ptr<TPrinter> Prntr(new TPrinter); //like new
   Prntr->BeginDoc();
   try
   {
      for (int i = 0; i < count; ++i)
      {
         Prntr->Canvas->TextOut(
            200,
            200 + (i * Prntr->Canvas->TextHeight(sText->Strings[i])),
            sText->Strings[i]
         );
      }
      Prntr->Canvas->Brush->Color = clBlack;
   }
   __finally {
      Prntr->EndDoc();
   }
}
//---------------------------------------------------------------------------

void ZapisVRichEditWindow(std::unique_ptr<TStringList> &zapis)
{
   std::unique_ptr<TUpdateRichEdit> updateLB(new TUpdateRichEdit(zapis));
   updateLB->Notify();
   updateLB.release();
}
//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
   try
   {
      std::unique_ptr<TStringList> Sdata(new TStringList);
      AContext->Connection->IOHandler->ReadStrings(Sdata.get());
      ZapisVRichEditWindow(Sdata);
   }
   catch (const EIdConnClosedGracefully &)
   {
   }
   catch (const Exception &E)
   {
      String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
      //ShowMessage(mis);
      ::MessageBoxW(nullptr, mis.c_str(), Application->Title.c_str(), MB_OK | MB_ICONERROR);
      throw;
   }
}

/* alternatively:

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
   std::unique_ptr<TStringList> Sdata(new TStringList);
   AContext->Connection->IOHandler->ReadStrings(Sdata.get());
   ZapisVRichEditWindow(Sdata);
}

void __fastcall TFormPrint::IdTCPServer1Exception(TIdContext *AContext, Exception *AException)
{
   if (!dynamic_cast<EIdConnClosedGracefully*>(AException))
   {
      String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
      //ShowMessage(mis);
      ::MessageBoxW(nullptr, mis.c_str(), Application->Title.c_str(), MB_OK | MB_ICONERROR);
   }
}
*/

//---------------------------------------------------------------------------

void __fastcall TFormPrint::IdTCPServer1Connect(TIdContext *AContext)
{
   //for russian
   AContext->Connection->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();
}
//---------------------------------------------------------------------------


Code: Select all

void __fastcall TFormSent::Button1Click(TObject *Sender)
{
   try
   {
      IdTCPClient1->Connect();
      try
      {
         IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
      }
      __finally
      {
         IdTCPClient1->Disconnect();
      }
   }
   catch (const Exception &E)
   {
      String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
      ShowMessage(mis);
   }
}


[Lena]

Thank you very much! I tried your code and it works fine. The printer is printing. Tested!
As usual a few questions
1. Do I need to add your code line:

Code: Select all

void __fastcall TFormPrint::IdTCPServer1Execute(TIdContext *AContext)
{
   try
   {
      std::unique_ptr<TStringList> Sdata(new TStringList);
      AContext->Connection->IOHandler->ReadStrings(Sdata.get());
      ZapisVRichEditWindow(Sdata);
     //AContext->Connection->Disconnect(); <----This one?
***


2. If the Windows computer is turned off, the Android application does not respond for a long time. I set IdTCPClient1->ConnectTimeout in 5 seconds. Now everything looks good.
3. I replaced nullptr to NUII ([bcc32 Error] UnitPrint.cpp(133): E2451 Undefined symbol 'nullptr'). C++ Builder Berlin. Print Server application create for Win32.
4. Will there be thread safe when doing so?

Code: Select all

//IdTCPServer1Execute
catch (const EIdConnClosedGracefully &)
   {
   }
   catch (const Exception &E)
   {
      //here code to save an external file on disk log.txt
   }

5. Will I be able In the future to make the client application without code changes for IOS?

Code: Select all

void __fastcall TFormMenu::Button2Click(TObject *Sender)
{
 try
   {
     IdTCPClient1->Connect();
     try
     {
      IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
           ShowMessage(L"Thank you for your order!");
      Close(); //close TFormMenu
     }
      __finally
        {
         IdTCPClient1->Disconnect();
        }
   }
   catch (const Exception &E)
      {
       String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
        ShowMessage(mis);
       }
}


6. Files like: #include <Vcl.Printers.hpp> #include <memory>...
where it is better to insert before or after #pragma hdrstop or without a difference?

[rlebeau]

1. Do I need to add your code line:

Code: Select all

     //AContext->Connection->Disconnect(); <----This one?
***


If you absolutely require the client to disconnect after just 1 string is sent, then you could add it, yes. Or just let the server throw an exception when the client disconnects. But, if possible, I would suggest changing the client to not disconnect after each string, reuse a connection as much as feasible. There is overhead in (re)connecting TCP connections.

3. I replaced nullptr to NUII ([bcc32 Error] UnitPrint.cpp(133): E2451 Undefined symbol 'nullptr'). C++ Builder Berlin.

nullptr was added in C++11. The classic 32bit compiler (bcc32) is not a C++11 compiler. The clang 32bit compiler (bcc32c) is, as is the Android compiler.

4. Will there be thread safe when doing so?

Code: Select all

//IdTCPServer1Execute
catch (const EIdConnClosedGracefully &)
   {
   }
   catch (const Exception &E)
   {
      //here code to save an external file on disk log.txt
   }

That is fine, provided that the code which accesses the file is thread-safe, such as wrapping it with a critical section or mutex.

5. Will I be able In the future to make the client application without code changes for IOS?

Firemonkey and Indy work (roughly) the same way in iOS as they do in Android. So there will be little code change needed.

But, even in Android, you shouldn't be making TCP connections in the main UI thread at all. That is a good way to get your app killed by a mobile OS. Move the TCP connection to a worker thread instead. That will also better facilitate you being able to reuse the connection for sending multiple strings without disconnecting in between.

6. Files like: #include <Vcl.Printers.hpp> #include <memory>...
where it is better to insert before or after #pragma hdrstop or without a difference?

There is a difference. Read up on how "Precompiled Headers" work.

[Lena]

But, if possible, I would suggest changing the client to not disconnect after each string, reuse a connection as much as feasible.

Do You mean I have to do so for the restaurant business?

Code: Select all

//second form FormMenu
void __fastcall TFormMenu::Button2Click(TObject *Sender)
{
 try
   {
      IdTCPClient1->Connect();
      IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
           ShowMessage(L"Thank You for order!");
      Close();
}
   }
   catch (const Exception &E)
      {
       String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
       ShowMessage(mis);
       }
}
//****


//close main form
void __fastcall TForm1::FormClose(TObject *Sender, TCloseAction &Action)
{
 FormMenu->IdTCPClient1->Disconnect();
}


Is it correct?

[rlebeau]

Do You mean I have to do so for the restaurant business?

If that is your client app, yes.

Code: Select all

//second form FormMenu
void __fastcall TFormMenu::Button2Click(TObject *Sender)
{
 try
   {
      IdTCPClient1->Connect();
      IdTCPClient1->IOHandler->Write(Memo1->Lines, true, IndyTextEncoding_UTF8());
           ShowMessage(L"Thank You for order!");
      Close();
}
   }
   catch (const Exception &E)
      {
       String mis = _D("Error class: ") + E.ClassName() + _D(" Error message: ") + E.Message;
       ShowMessage(mis);
       }
}
//****


//close main form
void __fastcall TForm1::FormClose(TObject *Sender, TCloseAction &Action)
{
 FormMenu->IdTCPClient1->Disconnect();
}


Is it correct?

No. Even if the FormMenu object is not freed when it is closed, you are still calling Connect() every time you want to send a new string. Call Connect() only if you are NOT ALREADY connected. Once connected, STAY connected.

I would suggest moving the TIdTCPClient to a background thread (which you have to do anyway if the client app is running on a mobile OS) and then have TFormMenu::Button2Click() delegate to that thread when sending, don't send in the main UI thread.

Something more like this:

Code: Select all

void __fastcall TForm1::TForm1(TComponent *Owner)
{
    Thread = new TMyThread;
}

void __fastcall TForm1::~TForm1()
{
    Thread->Terminate();
    Thread->WaitFor();
    delete Thread;
}

...

void __fastcall TFormMenu::Button2Click(TObject *Sender)
{
    Thread->Send(Memo1->Lines);
    ShowMessage(L"Thank You for order!");
    Close();
}

...

__fastcall TMyThread::~TMyThread()
{
    ClearQueue();
}

void __fastcall TMyThread::Send(TStrings *Text)
{
    CopyAndAddToQueue(Text);
}

void __fastcall TMyThread::Execute()
{
    while (!Terminated)
    {
        try
        {
            try
            {
                Client->Connect();
            }
            catch (const Exception &e)
            {
                // notify main UI ...

                for(int i = 0; (i < 5) && !Terminated; ++i)
                    Sleep(1000);

                continue;
            }

            try
            {
                Client->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();

                while (!Terminated)
                {
                    TStrings *Text = GetTextFromQueue();
                    if (Text)
                    {
                        try {
                            Client->IOHandler->Write(Text, true);
                        }
                        __finally {
                            delete Text;
                        }
                    }
                }
            }
            __finally
            {
                Client->Disconnect();
            }
        }
        catch (const Exception &e)
        {
            // notify main UI ...
        }
    }
}


[Lena]

Thank You.

would suggest moving the TIdTCPClient to a background thread

Sorry I do not quite understand your code please be more detailed:
1. Thread = new TMyThread;
For this File->New->Other->C++ Builder Files->Thead Object name=TMyThread
2. How is the declaration ClearQueue(); CopyAndAddToQueue(Text); GetTextFromQueue();?
Thank You very much.

[rlebeau]

Sorry I do not quite understand your code please be more detailed

It was PSEUDO-CODE intended to make you THINK about the problem and FILL IN the missing pieces in your own code.

1. Thread = new TMyThread;

Derive a class from TThread and override its virtual Execute() method.

For this File->New->Other->C++ Builder Files->Thead Object name=TMyThread

That will work (though I never use that IDE wizard in my own projects).

2. How is the declaration ClearQueue(); CopyAndAddToQueue(Text); GetTextFromQueue();?

Those are the pseudo pieces you were expected to fill in yourself.

<sigh> Apparently I have to write some actual code for you

Try something like this (untested, may need further tweaking):

Code: Select all

#include <System.Classes.hpp>

#include <list>
#include <memory>
#include <mutex>
#include <condition_variable>

class TThread
{
private
    std::list<std::unique_ptr<TStringList>> Queue;
    std::mutex QueueLock;
    std::condition_variable QueueEvent;

protected:
    void __fastall Execute() override;
    void __fastcall TerminatedSet() override;

public:
    __fastcall TMyThread();

    void Send(TStrings *Text);
    void Send(std::unique_ptr<TStringList> &&Text);
};


Code: Select all

#include <IdTCPClient.hpp>
#include <thread>
#include <chrono>

__fastcall TMyThread::TMyThread()
    : TThread(false)
{
}

void __fastcall TMyThread::Execute()
{
    // create and configure TCP client as needed...
    std::unique_ptr<TIdTCPClient> Client(new TIdTCPClient);

    Client->Host = ...;
    Client->Port = ...;
    Client->ConnectTimeout = 5000;

    do
    {
        if (Terminated) return;

        try
        {
            // try to connect to the server...

            try
            {
                Client->Connect();
            }
            catch (const Exception &e)
            {
                // notify main UI as needed ...

                // wait some time before retrying ...
                for (int i = 0; i < 5; ++i)
                {
                    if (Terminated) return;
                    std::this_thread::sleep_for(std::chrono::seconds(1));
                }

                continue;
            }

            // connected!

            try
            {
                Client->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();

                std::unique_lock<std::mutex> lk(QueueLock);
                do
                {
                    if (Terminated) return;

                    if (Queue.empty())
                    {
                        // wait up to 15 minutes for new messages to send again ...

                        if (!QueueEvent.wait_for(lk, std::chrono::minutes(15), [this](){ return (!Queue.empty()) || Terminated; }))
                        {
                            // 15 minutes elapsed, disconnect and wait for new messages before reconnecting ...
                            break;
                        }

                        if (Terminated) return;
                    }

                    // move the main Queue to a temp copy and clear the main Queue,
                    // then unlock access to the main Queue so it can receive new messages
                    // while sending the copy...

                    std::list<std::unique_ptr> TmpQueue = std::move(Queue);
                    lk.unlock();

                    for (auto &Text : TmpQueue )
                    {
                        if (Terminated) return;
                        Client->IOHandler->Write(Text.get(), true);
                    }

                    // send finished, re-acquire the main Queue lock ...
                    lk.lock();
                }
                while (true);
            }
            __finally
            {
                Client->Disconnect();
            }

            // if we get this far, a 15 minute idle period elapsed and the
            // connection was closed, so wait for new messages to send
            // before reconnecting ...

            if (Terminated) return;

            std::unique_lock<std::mutex> lk(QueueLock);
            if (Queue.empty())
                QueueEvent.wait(lk, [this](){ return (!Queue.empty()) || Terminated; });
        }
        catch (const Exception &e)
        {
            // notify main UI as needed ...
        }
    }
    while (true);
}

void TMyThread::Send(TStrings *Text)
{
    // make a copy of the strings and add the copy to the main Queue ...
    std::unique_ptr<TStringList> Copy(new TStringList);
    Copy->Assign(Text);
    Send(std::move(Copy));
}

void TMyThread::Send(std::unique_ptr<TStringList> &&Text)
{
    // take ownership of the strings and add them to the main Queue ...
    std::unique_lock<std::mutex> lk(QueueLock);
    Queue.push_back(Text);
    QueueEvent.notify_one();
}

void __fastcall TMyThread::TerminatedSet()
{
    // signal the conditional variable in case the thread is still waiting on it ...
    QueueEvent.notify_one();
}


Code: Select all

void __fastcall TForm1::TForm1(TComponent *Owner)
{
    Thread = new TMyThread;
}

void __fastcall TForm1::~TForm1()
{
    Thread->Terminate();
    Thread->WaitFor();
    delete Thread;
}

...

void __fastcall TFormMenu::Button2Click(TObject *Sender)
{
    Thread->Send(Memo1->Lines);
    ShowMessage(L"Thank You for order!");
    Close();
}


Alternatively, using Embarcadero RTL classes instead of C++11 classes:

Code: Select all

#include <System.Classes.hpp>
#include <System.SyncObjs.hpp>
#include <System.Generics.Collections.hpp>
#include <memory>

class TThread
{
private
    std::unique_ptr<TObjectList<TStringList>> Queue;
    std::unique_ptr<TCriticalSection> QueueLock;
    std::unique_ptr<TEvent> QueueEvent;

protected:
    void __fastall Execute() override;
    void __fastcall TerminatedSet() override;

public:
    __fastcall TMyThread();

    void Send(TStrings *Text);
};


Code: Select all

#include <IdTCPClient.hpp>

__fastcall TMyThread::TMyThread() :
    TThread(false),
    Queue(new TObjectList<TStringList>(true)),
    QueueLock(new TCriticalSection),
    QueueEvent(new TEvent(NULL, true, false, ""))
{
}

void __fastcall TMyThread::Execute()
{
    // create and configure TCP client as needed...
    std::unique_ptr<TIdTCPClient> Client(new TIdTCPClient);

    Client->Host = ...;
    Client->Port = ...;
    Client->ConnectTimeout = 5000;

    do
    {
        if (Terminated) return;

        try
        {
            // try to connect to the server...

            try
            {
                Client->Connect();
            }
            catch (const Exception &e)
            {
                // notify main UI as needed ...

                // wait some time before retrying ...
                for (int i = 0; i < 5; ++i)
                {
                    if (Terminated) return;
                    Sleep(1000);
                }

                continue;
            }

            // connected!

            try
            {
                Client->IOHandler->DefStringEncoding = IndyTextEncoding_UTF8();

                do
                {
                    if (Terminated) return;

                    if (QueueEvent->WaitFor(15000) != TWaitResult::wrSignaled)
                    {
                        // 15 minutes elapsed, disconnect and wait for new messages before reconnecting ...
                        break;
                    }

                    // move the main Queue to a temp copy and clear the main Queue,
                    // then unlock access to the main Queue so it can receive new messages
                    // while sending the copy...

                    std::unique_ptr<TObjectList<TStringList>> TmpQueue(new TObjectList<TStringList>(true));

                    QueueLock->Enter();
                    try
                    {
                        TmpQueue.swap(Queue);
                        QueueEvent->ResetEvent();
                    }
                    __finally
                    {
                        QueueLock->Leave();
                    }

                    int count = TmpQueue->Count;
                    for (int i = 0; i < count; ++i)
                    {
                        if (Terminated) return;
                        Client->IOHandler->Write(TmpQueue->Items[i], true);
                    }
                }
                while (true);
            }
            __finally
            {
                Client->Disconnect();
            }

            // if we get this far, a 15 minute idle period elapsed and the
            // connection was closed, so wait for new messages to send
            // before reconnecting ...

            if (Terminated) return;

            QueueEvent->WaitFor(INFINITE);
        }
        catch (const Exception &e)
        {
            // notify main UI as needed ...
        }
    }
    while (true);
}

void TMyThread::Send(TStrings *Text)
{
    // make a copy of the strings and add the copy to the main Queue ...
    std::unique_ptr<TStringList> Copy(new TStringList);
    Copy->Assign(Text);
    QueueLock->Enter();
    try
    {
        Queue->Add(Copy.get());
        Copy.release();
        QueueEvent->SetEvent();
    }
    __finally
    {
        QueueLock->Leave();
    }
}

void __fastcall TMyThread::TerminatedSet()
{
    // signal the conditional variable in case the thread is still waiting on it ...
    QueueEvent->SetEvent();
}


[Lena]

Thank you very much for the code! The code is very complicated. I will try to understand this.