[LinuxCNC/linuxcnc PR#337] renew

#1 – jasenk2 于 2017-10-02

Opps, it some old sserial.c here sorry

2017-10-02 23:47 GMT+03:00 Sebastian Kuzminsky :

> @SebKuzminsky requested changes on this pull request.
>
> This PR needs significant work before it can be merged.
>
> – The issues identified as comments in the diff need to be addressed.
> – The name of the component should be simplified, and should not
> include the name of the author.
> – The merge commits should be rebased out.
>
> ——————————
>
> In src/hal/drivers/mesa-hostmot2/sserial.c
> <https://github.com/LinuxCNC/linuxcnc/pull/337#discussion_r142249253>:
>
> > @@ -1344,14 +1344,14 @@ void hm2sserialpreparetramwrite(hostmot2_t *hm2, long period){
> double temp = *pin->float_pin;
> memcpy(&buff, &temp, sizeof(double));
> } else {
> – HM2ERRNOLL(“sserial write: LBPFLOAT of bit-length %i not handled\n”, conf->DataLength);
> – conf->DataType = 0; // only warn once, then ignore
> + HM2ERRNOLL(“sserial read: LBPFLOAT of bit-length %i not handled\n”, conf->DataLength);
>
> Why change this from “write” to read”? It’s in the
> hm2sserialpreparetramwrite() function, i think the original message
> is correct.
> ——————————
>
> In .gitignore
> <https://github.com/LinuxCNC/linuxcnc/pull/337#discussion_r142249355>:
>
> > @@ -40,4 +40,6 @@ oprofile*
> *.log
> position.txt
> *.9
> -*.glade.h
>
> Why did you remove this line?
> ——————————
>
> In src/hal/drivers/mesa-hostmot2/sserial.c
> <https://github.com/LinuxCNC/linuxcnc/pull/337#discussion_r142249487>:
>
> > @@ -1090,7 +1090,7 @@ int hm2sserialcreatepins(hostmot2t hm2, hm2sserialremote_t chan){
> if (r < 0) { > HM2_ERR(“error adding pin ‘%s’, aborting\n”, name);
> return r;
> – }
> + };
>
> No need for a semicolon here.
> ——————————
>
> In src/hal/drivers/mesa-hostmot2/sserial.c
> <https://github.com/LinuxCNC/linuxcnc/pull/337#discussion_r142249835>:
>
> > @@ -1534,14 +1534,12 @@ int hm2sserialreadpins(hm2sserialremotet *chan){
> *pin->float_pin = (double)(pin->accum – pin->offset) / pin->fullscale ;
> break;
> case LBP_FLOAT:
> – if (conf->DataLength == sizeof(float) * 8){
> – float temp;
> – memcpy(&temp, &buff, sizeof(float));
> – *pin->float_pin = temp;
> + if (conf->DataLength == sizeof(float) * 8 ){
> + float temp = *pin->float_pin;
> + memcpy(&buff, &temp, sizeof(float));
>
> The new logic and the new indentation here are both wrong.
> ——————————
>
> In src/hal/drivers/mesa-hostmot2/sserial.c
> <https://github.com/LinuxCNC/linuxcnc/pull/337#discussion_r142249956>:
>
> > } else if (conf->DataLength == sizeof(double) * 8){
> – double temp;
> – memcpy(&temp, &buff, sizeof(double));
> – *pin->float_pin = temp;
> + double temp = *pin->float_pin;
> + memcpy(&buff, &temp, sizeof(double));
>
> The new logic and indentation here are both wrong.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/pull/337#pullrequestreview-66584557>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ACATkYT8szgwoOp7f6c3FNr7RmdGvX77ks5soUvOgaJpZM4PrN2H>
> .
>

#2 – jasenk2 于 2017-10-02

All other is looking OK for now

#3 – jasenk2 于 2017-10-04

all request are done