On Monday, December 18, 2017 03:00:47 Binghoo Dang via Digitalmars-d-learn
wrote:
> hi,
>
> I'm a dlang newbie, and I recently made a program that interacts
> with an RFID Reader, it's basically a Serial-Port communication
> model. And after 2 days, I finally got a working code like these:
>
> ```
> enum RCPCmd{
> RCPCMD_GET_REGION = 0x06,
> RCPCMD_SET_REGION = 0x07,
> RCPCMD_SYS_RESET = 0x08,
> ...........
>
> RCPCMD_START_AUTOREAD2 = 0x36,
> RCPCMD_STOP_AUTOREAD2 = 0x37,
>
> RCPCMD_WRITE_TYPEC_TAGDATA = 0x46,
>
> RCPCMD_FAIL = 0xFF
> }
>
> enum {
> RCPMSG_CMD = 0x00,
> RCPMSG_RESP = 0x01,
> RCPMSG_NOTIFY = 0x02
> }
>
> private struct RCProtocol
> {
> enum {
> RCPPKT_PRE = 0xBB,
> RCPPKT_END = 0x7E
> }
>
> align(1) struct RCPacketHdr {
> ubyte msgtype;
> ubyte cmdcode;
> ushort len;
> }
>
> /**
> * These kind of members will be need only for private
> var for making a cmd packet.
> * But access-able as a parsed result for a incomming
> packet.
> */
> ubyte preamble;
> RCPacketHdr hdr;
> ubyte[] payload; //ubyte[len] of payload.
> ubyte end;
> ushort crc16;
>
> ubyte[] makeCMDPacket(RCPCmd cmd, in ubyte[] data)
> {
> this.preamble = RCPPKT_PRE;
> this.hdr.msgtype = RCPMSG_CMD;
> this.hdr.cmdcode = cast (ubyte) cmd;
> this.hdr.len = 0xffff & data.length;
Why are you using & instead of simply casting to ushort? Casting would be
more idiomatic. Or you can use to!ushort if you want to verify the size at
runtime and have a ConvException thrown if the value is too large.
> this.end = RCPPKT_END;
> this.payload = data.dup;
>
> ubyte [] pkt;
> pkt ~= this.hdr.msgtype;
> pkt ~= this.hdr.cmdcode;
> pkt ~= this.hdr.len >> 8;
> pkt ~= this.hdr.len & 0xff;
> pkt = this.preamble ~ pkt ~ this.payload ~ this.end;
>
> // calc crc16 for Preamble + HDR + Data + END
> this.crc16 = new RCPCRC16().calc(pkt);
>
> pkt ~= this.crc16 >> 8;
> pkt ~= this.crc16 & 0xff;
>
> return pkt;
> }
>
> int parseRespPacket(in ubyte[] recv)
> {
> const size_t len = recv.length;
> if (recv is null)
> return PARSE_ERR;
>
> if (len < 8)
> return PARSE_ERR_PKTLEN;
>
> if (recv[0] !is RCPPKT_PRE)
> return PARSE_ERR_PKTHEAD;
>
> this.preamble = recv[0];
> this.hdr.msgtype = recv[1];
> this.hdr.cmdcode = recv[2];
> this.hdr.len = (recv[3] << 8) + 0xff & recv[4];
>
> if ( this.hdr.len > (len - 8))
> return PARSE_ERR_PKT;
>
> this.end = recv[5+this.hdr.len];
>
> if (this.end != RCPPKT_END)
> return PARSE_ERR_PKT;
>
> this.payload = recv[5 .. (5+this.hdr.len)].dup;
>
> ubyte [] pkt;
> pkt ~= this.hdr.msgtype;
> pkt ~= this.hdr.cmdcode;
> pkt ~= this.hdr.len >> 8;
> pkt ~= this.hdr.len & 0xff;
> pkt = this.preamble ~ pkt ~ this.payload ~ this.end;
>
> // calc crc16 for Preamble + HDR + Data + END
> const ushort desired_crc = new RCPCRC16().calc(pkt);
>
> this.crc16 = (recv[6+this.hdr.len] << 8) +
> recv[7+this.hdr.len];
> if (this.crc16 != desired_crc) {
> writefln("-- PARSE ERR: CRC, desired = %04X,
> actuall is %04X", this.crc16, desired_crc);
> return PARSE_ERR_CRC;
> }
>
> return PARSE_OK;
> }
> }
>
> ```
>
> It's basically a C-style code, and it works perfect, But I know
> that this is not a "D-way", So, any suggestions?
I'd suggest that you check out std.bitmanip if you haven't.
https://dlang.org/phobos/std_bitmanip.html
append, peek, read, and write in particular are useful if you're putting
various integral values into an array of ubyte[] or reading them from it.
Glancing over what you have, I'm pretty sure that all of the bitshifts and
bitwise &'s and can be removed by using append or write to write the data to
the array and peek or read to read from it.
Also, as far as D style goes, variables and enum values are usually given
camelCase names, whereas you've named your enum values in all uppercase and
your variables names in all lowercase, and you've underscores in the middle
of names. But of course, you can do what you want on that. It's just that if
you make any libraries public (e.g. on code.dlang.org), it's better to
follow the D naming guidelines for anything in the public API:
https://dlang.org/dstyle.html
- Jonathan M Davis
