Skip to content
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

Added support for bytesize, stopbits, parity #34

Closed
wants to merge 3 commits into from

Conversation

paocalvi
Copy link

I try to add support for bytesize 5,6,7,8, parity, stopbits.
I tested on windows and it seems to work.
I don't know how to deal with the 1.5 stop bits, so I turn it on-the-fly to 2
I cannot compile the "posix" version so I can't guaranteed about that.
Each new parameter is optional, so the library is retro-compatible and defaulted to 8N1, as requested by the author.

Calvi Paolo and others added 3 commits November 28, 2015 16:37
Tested on Windows.
Notes:
1.5 stop bits not implemented, using 2 if 1.5 set
1.5 stop bits not implemented, using 2 instead
tested on windows, by now
@tarm
Copy link
Owner

tarm commented Dec 10, 2015

Hi Calvi,

Thanks for this PR! It's on my radar and I would like to merge it, but also want to do some testing on different platforms.

I hope to be able to do that in the next 2 weeks. I know that is a long time.

@paocalvi
Copy link
Author

Don't worry.
I tested that on Windows on a 4800, 8E1 serial porte and I discovered the
bug which I corrected in my last commit.
I hope I can test on Linux soon., also.
Thanks for the great job.

Paolo

On Thu, Dec 10, 2015 at 7:09 AM, tarm notifications@github.com wrote:

Hi Calvi,

Thanks for this PR! It's on my radar and I would like to merge it, but
also want to do some testing on different platforms.

I hope to be able to do that in the next 2 weeks. I know that is a long
time.


Reply to this email directly or view it on GitHub
#34 (comment).

@tajtiattila
Copy link
Contributor

Thank you for this PR, I just realized I need these settings for my project.

Without this the serial package will reset the correct settings even if they were properly set by the user by other means (eg. MODE command on Windows).

I think DataBits should simply be a byte, there is no need for a new type. Just document the fact a zero value means 8 bits. The package-internal platform specific openPort function could then just fail with an error when an invalid value was present. Perhaps it should do just that for StopBits15 on linux until someone figures out how to do it right.

I would drop the "Type" bit from the new type names, and change the values to use CamelCase according to those in standard libs (eg. md5.BlockSize, os.PathSeparator). Furthermore they could be simply added to serial.go instead of having them in a separate file.

const DefaultSize = 8 // default value for Config.Size if it is 0

type StopBits byte
const (
    StopBits1 StopBits = iota
    StopBits15
    StopBits2
)

type Parity byte
const (
    ParityNone Parity = iota
    ParityOdd
    ParityEven
    ParityMark
    ParitySpace
)

@tajtiattila
Copy link
Contributor

I added my own version based on the work from Paolo in commit 2a4c27d to https://github.com/tajtiattila/serial.

@tajtiattila
Copy link
Contributor

I added some changes to my fork, please feel free to comment.

Notable changes:

  • Parity and StopBits are not based on to the values in Windows anymore
  • Parity is still a byte, and is the first char of the parity name (of "NOEMS")
  • StopBits is 1 or 2 or 15, the constant Stop15 was renamed to Stop1Half
  • replaced some naked returns with arguments (naked returns being bad practice according to Brad/Andrew)
  • added Config.String() and ParseConfig() along with tests

@paocalvi
Copy link
Author

Great job. During next week I will try to convert my code (which is now
based on my commit) to use yours, and I will test it on windows 115200 8N1
and 4800 8E1.
Testing on linux will be somehow trick, for me.

On Sat, Dec 12, 2015 at 12:44 PM, Tajti Attila notifications@github.com
wrote:

I added some changes to my fork, please feel free to comment.

Notable changes:

  • Parity and StopBits are not based on to the values in Windows anymore
  • Parity is still a byte, and is the first char of the parity name (of
    "NOEMS")
  • StopBits is 1 or 2 or 15, the constant Stop15 was renamed to
    Stop1Half
  • replaced some naked returns with arguments (naked returns being bad
    practice according to Brad/Andrew)
  • added Config.String() and ParseConfig() along with tests


Reply to this email directly or view it on GitHub
#34 (comment).

@tajtiattila
Copy link
Contributor

Thank you! I only have only an Arduino Uno for testing, 9600 Baud 8N1 worked with it.

One thing I'm not statisfied with in my version is the stop bit handling, and the actual constant names and values used. Basically I removed the 1.5 -> 2 stop bits conversion form linux/posix. This seems the correct thing to do, but is too restrictive. Perhaps one should be able to specify more than one stop bit (that is, 1.5 or more). Unfortunately all my knowledge of the serial port comes from Wikipedia.

@paocalvi
Copy link
Author

Well, 1.5 stop bits is actually 2 stop bits when transmitting and 1 when
receving.
Apparently it is not supported in the linux/posix structure, since the port
opening is done only once and you have to choose between 1 and 2....
Fortunately I guess nobody is using that anymore.... it is not a great
loss. As the "mark"/"space" parities, think....

On Sat, Dec 12, 2015 at 9:00 PM, Tajti Attila notifications@github.com
wrote:

Thank you! I only have only an Arduino Uno for testing, 9600 Baud 8N1
worked with it.

One thing I'm not statisfied with in my version is the stop bit handling,
and the actual constant names and values used. Basically I removed the 1.5
-> 2 stop bits conversion form linux/posix. This seems the correct thing to
do, but is too restrictive. Perhaps one should be able to specify more than
one stop bit (that is, 1.5 or more). Unfortunately all my knowledge of the
serial port comes from Wikipedia.


Reply to this email directly or view it on GitHub
#34 (comment).


const (
DATABITS_8 DataBitsType = iota
DATABITS_7
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to make DATABITS_7 = 7, etc.

Obviously 0 should be handled specially and get mapped to 8.

@tarm
Copy link
Owner

tarm commented Dec 15, 2015

I agree with a most of the comments from @tajtiattila

Can you coordinate a single updated PR with him (not sure if you want to update this PR or use his branch)?

Also could you please squash the 3 commits into a single commit?

tajtiattila referenced this pull request in tajtiattila/serial Dec 17, 2015
Based on the work of Calvi Paolo <calvip@it.mobile.marconi.com>
tarm pushed a commit that referenced this pull request Jan 31, 2016
This is discussed in these PRs:
#34
#38
@tarm
Copy link
Owner

tarm commented Jan 31, 2016

I merged the first 3 commits from #38 which provide this same functionality. Thanks Calvi!

@tarm tarm closed this Jan 31, 2016
TopOneExpert added a commit to TopOneExpert/go_serial-port that referenced this pull request Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants