Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow output precision to be specified for socket output #579

Merged
merged 1 commit into from
Feb 8, 2022
Merged

Allow output precision to be specified for socket output #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/input_output/FGOutputSocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ bool FGOutputSocket::Load(Element* el)
el->GetAttributeValue("protocol") + "/" +
el->GetAttributeValue("port"));

// Check if output precision for doubles has been specified, default to 7 if not
if(el->HasAttribute("precision"))
precision = (int)el->GetAttributeValueAsNumber("precision");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the format width setw is still 12 characters wide, I guess we should make sure that precision is no higher than, say 10 (reserving storage for 1 digit and the decimal point) ?
If precision is beyond this value then either it is ignored or it is capped. A warning message should be issued as well when JSBSim alters the user input value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was writing the code I half wondered about letting the user specify the width as well as the precision to give them full control.

I must say reading the documentation on setw and setprecision it wasn't entirely clear what the exact output would be especially given various combinations, and with comments like this in the C++ documentation.

The exact effects this modifier has on the input and output vary between the individual I/O functions and are described at each operator<< and operator>> overload page individually.

I assumed setprecision would set the number of places after the decimal point, but it doesn't, e.g. from the C++ documentation - https://en.cppreference.com/w/cpp/io/manip/setprecision

default precision (6): 3.14159
std::setprecision(10): 3.141592654

So I ran a couple of tests with various combinations:

double test = 123456789.12345678912345;

setw	setprecision	output			strlen
12	10		" 123456789.1"		12
12	12		"123456789.123"		13
12	14		"123456789.12346"	15


double test = 56789123456789.12345678912345;

setw	setprecision	output			strlen
12	10		"5.678912346e+13" 	15
12	12		"5.67891234568e+13"	17
12	14		"56789123456789"	14

I'm happy to add some code to apply a capping on the precision relative to the width, just not sure on the best logic to apply though 😉

Capping it at 10 is a safe option, users will need to realise that 10 doesn't mean 10 places after the decimal point, and in the first example there will be what they may think is 'wasted' padding. And for the second example it doesn't guarantee that the overall width will be 12.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reading the C++ docs and testing, I wasn't aware of all those subtleties 👍

Your tests clearly show that if we want the string length to stay below 12 characters then a number of places must be allocated in the string for:

  • the leading minus sign -
  • the decimal point .
  • the exponent e
  • the exponent sign + or -
  • the exponent value: 2 figures max ?

That sums up to 6 characters. So keeping the string length below 12 characters in all cases means that precision must be lower than or equal to 6 characters meaning that, even with the current code, numbers can be more than 12 characters long.

So the question is about knowing if issuing to a socket connection numbers that are more than 12 characters long is a problem ? I have not inspected the code but I guess that it's the socket receivers that might make such assumptions, not JSBSim as an emitter.

To stay on the safe side, I would suggest to disable this feature for FlightGear sockets (i.e. to force precision to 7 in FGOutputFG and to issue a warning message whenever the user specifies a different value) in case FlightGear makes some assumptions about the numbers length.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and I agree that checking precision against 10 is irrelevant to the general case FGOutputSocket. But that opens the question about whether or not the user should be allowed to modify the width setw ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stay on the safe side, I would suggest to disable this feature for FlightGear sockets (i.e. to force precision to 7 in FGOutputFG

Hmm, I'm a bit confused about the FlightGear comment with regards to precision since when I took a quick look it looks like the FlightGear socket outputs a binary data structure containing binary floats and doubles.

Whereas the precision we're dealing with here is for a text representation with a certain amount of precision.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To stay on the safe side, I would suggest to disable this feature for FlightGear sockets (i.e. to force precision to 7 in FGOutputFG

Hmm, I'm a bit confused about the FlightGear comment with regards to precision since when I took a quick look it looks like the FlightGear socket outputs a binary data structure containing binary floats and doubles.

Whereas the precision we're dealing with here is for a text representation with a certain amount of precision.

Err.. well, it was indeed nonsense. Sorry !

else
precision = 7;

return true;
}

Expand All @@ -130,7 +136,7 @@ bool FGOutputSocket::InitModel(void)
{
if (FGOutputType::InitModel()) {
delete socket;
socket = new FGfdmSocket(SockName, SockPort, SockProtocol);
socket = new FGfdmSocket(SockName, SockPort, SockProtocol, precision);

if (socket == 0) return false;
if (!socket->GetConnectStatus()) return false;
Expand Down
1 change: 1 addition & 0 deletions src/input_output/FGOutputSocket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class FGOutputSocket : public FGOutputType
unsigned int SockPort;
FGfdmSocket::ProtocolType SockProtocol;
FGfdmSocket* socket;
int precision;
};
}
//%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down
8 changes: 5 additions & 3 deletions src/input_output/FGfdmSocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ static bool LoadWinSockDLL(int debug_lvl)
}
#endif

FGfdmSocket::FGfdmSocket(const string& address, int port, int protocol)
FGfdmSocket::FGfdmSocket(const string& address, int port, int protocol, int precision)
{
sckt = sckt_in = 0;
Protocol = (ProtocolType)protocol;
connected = false;
struct addrinfo *addr = nullptr;
this->precision = precision;

#if defined(_MSC_VER) || defined(__MINGW32__)
if (!LoadWinSockDLL(debug_lvl)) return;
Expand Down Expand Up @@ -150,12 +151,13 @@ FGfdmSocket::FGfdmSocket(const string& address, int port, int protocol)

//%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
// assumes TCP or UDP socket on localhost, for inbound datagrams
FGfdmSocket::FGfdmSocket(int port, int protocol)
FGfdmSocket::FGfdmSocket(int port, int protocol, int precision)
{
sckt = -1;
connected = false;
Protocol = (ProtocolType)protocol;
string ProtocolName;
this->precision = precision;

#if defined(_MSC_VER) || defined(__MINGW32__)
if (!LoadWinSockDLL(debug_lvl)) return;
Expand Down Expand Up @@ -338,7 +340,7 @@ void FGfdmSocket::Append(const char* item)
void FGfdmSocket::Append(double item)
{
if (buffer.tellp() > 0) buffer << ',';
buffer << std::setw(12) << std::setprecision(7) << item;
buffer << std::setw(12) << std::setprecision(precision) << item;
}

//%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
Expand Down
7 changes: 3 additions & 4 deletions src/input_output/FGfdmSocket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ CLASS DECLARATION
class FGfdmSocket : public FGJSBBase
{
public:
FGfdmSocket(const std::string& address, int port)
: FGfdmSocket(address, port, ptTCP) {}
FGfdmSocket(const std::string&, int, int);
FGfdmSocket(int, int);
FGfdmSocket(const std::string& address, int port, int protocol, int precision = 7);
FGfdmSocket(int port, int protocol, int precision = 7);
~FGfdmSocket();
void Send(void);
void Send(const char *data, int length);
Expand Down Expand Up @@ -104,6 +102,7 @@ class FGfdmSocket : public FGJSBBase
struct sockaddr_in scktName;
struct hostent *host;
std::ostringstream buffer;
int precision;
bool connected;
void Debug(int from);
};
Expand Down