串行端口写入缓冲器
我正在写入一些串行通信端口，并将特定的字节设置为缓冲区。我的写缓冲区协议写在下面。平台为Windows，开发环境为Visual C++。
- 字节0将有消息类型指示符(字节)
- 字节1-4将有消息号(Uint32)。
- 字节5-8将具有unix时代时间(Uint32)。
- 字节13-14将有样本数据包数(Uint16)。
- 字节15+ (最多60)将有示例数据包(Uint16)
- 字节15+(示例号*2)将有消息结束(字节)
现在，我关注的是从整数或短值构建缓冲区(原始字节)。
void ConvertIntToByte(int iValue, BYTE* ucRawBuffer, int iOffset)
{
ucRawBuffer[iOffset] = ((iValue >> 24) & 0xFF);
ucRawBuffer[iOffset+1] = ((iValue >> 16) & 0xFF);
ucRawBuffer[iOffset+2] = ((iValue >> 8) & 0xFF);
ucRawBuffer[iOffset+3] = (iValue & 0xFF);
}
void ConvertShortToByte(short iValue, BYTE* ucRawBuffer, int iOffset)
{
ucRawBuffer[iOffset] = (iValue >> 8) & 0xFF;
ucRawBuffer[iOffset+1] = iValue & 0xFF;
}
void DataThreadFunction()
{
while(!m_bClosed)
{
DWORD dwRet = WaitForSingleObject(m_hDataSendEvent, INFINITE);
if(dwRet == WAIT_OBJECT_0)
{
ResetEvent(m_hDataSendEvent);
int iUTCTime = GetEpochTime();
m_iMsgSequence++;
ASSERT(m_iMaxSamples != 0);
BYTE ucXmtdat[60] = {0};
ucXmtdat[0] = 0x0B;
ConvertIntToByte(iUTCTime, ucXmtdat, 1);
ConvertIntToByte(m_iMsgSequence, ucXmtdat, 5);
ConvertIntToByte(m_iMaxSamples, ucXmtdat, 13);
int iOffset = 15;
// m_data - member vector populating from some other method
for(unsigned int i = 0; i < m_data.size(); i++)
{
short iCurrData = m_data[i];
ConvertShortToByte(iCurrData, ucXmtdat, iOffset);
iOffset += sizeof(unsigned short);
}
m_data.clear();
iOffset = 15+(m_iMaxSamples*2);
ucXmtdat[iOffset] = 0xCA;
bool bSuccess = m_objComm.WriteCommPort(ucXmtdat, 60);
}
}
}回答 2
Code Review用户
发布于 2016-02-16 04:20:24
我看到了一些可以帮助你改进你的程序的东西。
不要使用匈牙利符号
阅读这来解释为什么你不应该使用匈牙利符号。
在适当的地方使用无符号类型(
)
传递给ConvertShortToByte或ConvertIntToByte的值不太可能是有符号的值。由于符号扩展而移动时，有符号值与无符号值的待遇不同。在这种情况下，这几乎肯定不是我们所需要的。
从函数
返回有用的东西
如果转换函数返回的指针刚好超过当前值，则转换函数将更有用。和指针和索引相比，为什么不直接传递指针呢？重写后的内容如下：
BYTE *ConvertShortToByte(unsigned short value, BYTE* buffer)
{
*buffer++ = value >> 8;
*buffer++ = value;
return buffer;
}消除了“幻数”
这段代码有许多“魔术数字”，即未命名的常量，如0x0B、0xCA、60等。通常最好避免这种情况，并给这些常量取有意义的名称。这样，如果需要更改任何内容，您就不必对" 60“的所有实例进行代码搜索，然后尝试确定这个特定的60是否与所需的更改相关，或者它是否是具有相同值的其他常量。
使用对象
包对于对象来说是自然的，那么为什么不创建一个Packet类呢？有一种方法可以做到：
class Packet {
public:
Packet (unsigned seq, unsigned time, const std::vector<unsigned short>& data)
{
BYTE *buff = buffer;
*buff++ = MSG_TYPE;
buff = ConvertIntToByte(time, buff);
buff = ConvertIntToByte(seq, buff);
buff = ConvertIntToByte(0, buff); // filler
buff = ConvertShortToByte(data.size(), buff);
for (const auto item : data) {
buff = ConvertShortToByte(item, buff);
}
*buff++ = MSG_END;
}
size_t size() const { return 16 + (((buffer[13] << 8) + buffer[14]) << 1); }
static constexpr BYTE MSG_TYPE = 0x0B; // start of packet
static constexpr BYTE MSG_END = 0xCA; // end of packet
static constexpr size_t BUFF_SIZE = 60;
private:
BYTE buffer[BUFF_SIZE];
};如果您需要访问底层buffer，有几种方法可以做到这一点。首先，您可以简单地使buffer成为公共数据成员(或者将Packet设置为struct)。其次，您可以提供一个句柄，将const *引用返回到内部缓冲区。第三，您可以通过提供一个write成员函数来消除这种需求。第四，您可以声明缓冲区的位置，然后使用“place new”，这是通信协议的一个常见用法，其中包构建在固定位置缓冲区中。由于许多人似乎不熟悉“elaborate new”的用法，我将详细说明。
使用"placement new"
如果已经分配了缓冲区，并且希望在适当的位置创建一个Packet，则可以使用“place new”来完成：
BYTE ucXmtdat[60];
Packet *pkt = new (ucXmtdat) Packet(m_iMsgSequence, iUTCTime, m_data);这将在不分配进一步内存的情况下在Packet指定的位置创建ucXmtdat。请注意，在使用此表单时，完全取决于您是否确保内存区域确实足够大，并且在完成时完全由您来销毁该对象。在这种情况下，没有什么可做的，因为ucXmtdat (顺便说一句，它不是一个很好的名字)是本地的，当它超出范围时就会消失。
Code Review用户
发布于 2015-12-17 19:21:48
转换函数
这些可以简化成一堆。首先，您不需要& 0xFF部分。任务本身就会发生这种情况。这就要求我们：
void ConvertIntToByte(int iValue, BYTE* ucRawBuffer, int iOffset)
{
ucRawBuffer[iOffset] = iValue >> 24;
ucRawBuffer[iOffset+1] = iValue >> 16;
ucRawBuffer[iOffset+2] = iValue >> 8;
ucRawBuffer[iOffset+3] = iValue;
}接下来，我们可以做得更好。我们要做的就是按大端顺序将int写入字节缓冲区。这实际上是一项任务。如果你已经在大端机器上了，那就像：
void ConvertIntToByte(int iValue, BYTE* ucRawBuffer, int iOffset)
{
*reinterpret_cast<int*>(ucRawBuffer + iOffset) = iValue;
}在一台小终端机器上，你必须进行字节交换。关于这一点，有htonlhtonl：
void ConvertIntToByte(int iValue, BYTE* ucRawBuffer, int iOffset)
{
*reinterpret_cast<int*>(ucRawBuffer + iOffset) = htonl(iValue);
}我要在这里假设小安迪安。这里的优点是写一个短消息实际上是一回事，转换函数和字节交换函数之间的唯一区别是：
void ConvertShortToByte(short iValue, BYTE* ucRawBuffer, int iOffset)
{
*reinterpret_cast<short*>(ucRawBuffer + iOffset) = htons(iValue);
}所以让我们使用一个函数模板。此外，拥有ucRawBuffer和iOffset都是多余的。让我们取消偏移量：
short hton(short value) { return htons(value); }
int hton(int value) { return htonl(value); }
template <class T>
void ConvertToByte(T value, BYTE* buffer)
{
*reinterpret_cast<T*>(buffer) = hton(value);
}凉爽的。由于我们更改了签名，所以现在更改如下：
ConvertIntToByte(iUTCTime, ucXmtdat, 1);
ConvertIntToByte(m_iMsgSequence, ucXmtdat, 5);
ConvertIntToByte(m_iMaxSamples, ucXmtdat, 13);至：
ConvertToByte(iUTCTime, ucXmtdat + 1);
ConvertToByte(m_iMsgSequence, ucXmtdat + 5);
ConvertToByte(m_iMaxSamples, ucXmtdat + 13);命名约定
匈牙利的符号太糟糕了。你真的需要i来知道int iValue是一个int吗？不，这就是i的作用。类似地，BYTE*告诉您ucRawBuffer是一个无符号字符，您不需要uc。额外的字符不会给您带来任何好处，需要更长的时间来键入，并且如果您决定更改类型，则会导致今后的问题。
因此，上述情况实际上变成了：
ConvertToByte(utcTime, xmtDat + 1);
ConvertToByte(msgSequence, xmtDat + 5);
ConvertToByte(maxSamples, xmtDat + 13);未使用/很少使用的变量
bSuccess (-> success)未使用。
dwRet和iCurrData都只在声明后才在行上使用，而且只有一次。他们两个人都可以摆脱。例如，后者可以更改为：
ConvertByte(data[i], xmtDat + offset);
offset += sizeof(data[i]); // don't use unsigned short here, that's brittle重构
如果将内部部分分解成自己的功能，整个函数会读得更好。这样，外层是：
void DataThread() { // obviously it's a Function
while (!closed) {
if (WaitForSingleObject(dataSendEvent, INFINITE) == WAIT_OBJECT_0)) {
handleDataEvent();
}
}
}https://codereview.stackexchange.com/questions/92063
复制相似问题
腾讯云开发者
