-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
feat: move to big endian for network communications #45
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Nicolas Adenis-Lamarre <[email protected]>
// use case ? | ||
} | ||
|
||
void convertFromNetwork(DMDUtil::DMD::StreamHeader* o) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the counterpart is missing. Don't we need to ensure big endian when sending data?
Line 339 in fc29cd7
m_pDMDServerConnector->write_n(&streamHeader, sizeof(StreamHeader)); |
@@ -77,6 +77,21 @@ void DMDUTILCALLBACK LogCallback(DMDUtil_LogLevel logLevel, const char* format, | |||
fprintf(stderr, "%s\n", buffer); | |||
} | |||
|
|||
void convertFromNetwork(DMDUtil::DMD::Update* o) { | |||
// use case ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Update struct contains "bigger" data types as well. So I think a conversion would be required here, too:
libdmdutil/include/DMDUtil/DMD.h
Lines 84 to 100 in fc29cd7
struct Update | |
{ | |
Mode mode; | |
AlphaNumericLayout layout; | |
int depth; | |
uint8_t data[256 * 64 * 3]; | |
uint16_t segData[256 * 64]; // RGB16 or segment data | |
uint16_t segData2[128]; | |
bool hasData; | |
bool hasSegData; | |
bool hasSegData2; | |
uint8_t r; | |
uint8_t g; | |
uint8_t b; | |
uint16_t width; | |
uint16_t height; | |
}; |
But not all fields are used for all modes. So maybe we can optimize the conversion per mode.
There's no PR description, what is the reasoning behind the move? |
As commented in the code I didn't care about endianess. The assumption was that dmdserver runs as a service on the same machine as the clients. So endianess should not be an issue. |
@nadenislamarre uses dmd-play as client. Therefore, the current state of the PR seems to be sufficient. But if the client uses libdmdutil (VPX, PPUC, etc), I assume that big endian should be ensured as well when sending data. |
@mkalkbrenner just wanted to know why, don't want to be involved in decision making as an observer. |
No description provided.