Added SetDTR, SetRTS, and SetDTRRTS functions + FlowCtrl abstracted type

[linux4life798]

Two commits:

• Added SetDTR, SetRTS, and SetDTRRTS functions (should be safe)
• I changed SetFlowControl to consume a new FlowCtrl type (int), which might make help with self-documentation. (this would require previous users to type-cast)

Let me know what you think!

[ziutek]

Hello Craig!

FlowCtrl type looks good to me.

But I think that SetDTR, SetRTS parameters should be int, not bool.

My experience with programming microcontrollers shows that signals should be not a boolean type. See for example:

https://godoc.org/github.com/ziutek/emgo/egpath/src/nrf5/hal/gpio#Pin https://godoc.org/github.com/ziutek/emgo/egpath/src/stm32/hal/gpio#Pin

Int type helps a lot to perform calculations on such signals (generate or analyze) avoiding conversions.

[linux4life798]

Hi Michał. First off, thanks for the great FTDI lib contribution!

That is true, but since Go doesn't naturally support boolean evaluation/operators on int types, it could make for slightly less elegant code. I guess it really depends on your preference. To toggle signal in int space: Go sig = (sig & 1) ^ 1 vs C's sig = !sig The other reasons I went with bool was to make it somewhat self-documenting for novice users. It also helps that another serial lib uses bool, so hotswappability is nice. https://github.com/bugst/go-serial/blob/v1/serial.go#L34

Either way, I would like to add SetRTS and SetDTR functionality to your upstream branch, so I'm more than willing to change to int. I could see how this would be annoying to swap back and forth between bool and int, if you are using it with other libs.

Best,

Craig

[ziutek]

int gives you a lot more freedom. I know it from my own experience because my first approach was just use bool as GPIO pin state. After some time with using bool as signal it turned out to be a big mistake.

Document that RTS/DTR signal is set to least significant bit of sig and pass sig&1 to libftdi. This allows you to use simply sig^1 to toggle signal. In my experience this approach works very well and I use it in Emgo: https://godoc.org/github.com/ziutek/emgo/egpath/src/stm32/hal/gpio#Pin.Store

Using int makes much less confusion. You need explicit check its value in code eg: sig == 0 or sig != 0 that is much more readable than sig or !sig.

Some signals are active low (IIRC in case of UART: RTS and CTS are active low). What u.SetRTS(true) means in such case? What is value of RTS line?

For completeness we need also RTS and CTS (or GetRTS, GetCTS) methods. Suppose we have active low input signal X. How we should interpret if u.X() { or if u.GetX() {?

If you analyze Go runtime and standard library you see that Go authors use bool very carefully and don't abuse it for anything else than true or false really means.

[linux4life798]

Good points. Thanks for taking the time to explain! I will change that commit to use int and look into what functions are needed for Get* functionality. It didn't seem immediately obvious how you can poll those values using libFTDI.

[ziutek]

One more comment to FlowCtrl and iota. As with bool as a signal I also have bad experience with using iota for peripheral register bits. Yes, I have used it this way and this was another mistake.

Use iota only for things where the concrete values doesn't means anything, eg. you only want distinct values and at most they order matters (but in such case you still must document: "don't split or reorder this values").

If you define bits in peripheral register or some protocol use explicit values. This helps a lot when you debug and defends itself against code changes.

[linux4life798]

So, I'm guessing you would want me to change the enums in the following patch: https://github.com/linux4life798/ftdi/commit/a7abae19c1974aa2dfdc01e73dfe56e74a4a512d

Best,

Craig

[ziutek]

Yes I want.

But this is old project and there are still iotas there written my hand so I can live with them :)

[linux4life798]

Hah, sounds good!