|
In your Tester class lock using the object of your serial class (HASPClass::m_locker) and remove the locking inside your Query() function.
|
|
|
|
|
Thanks for suggested improvement. (PS in your case I'd need to make HASPClass lock public?)
Otherwise my case is also correct right?
|
|
|
|
|
It must be public then.
Your case is correct if Tester is the only class using the serial class (as answered by Daniel).
But if you have another class that uses the serial communication from within another thread too, it would be not safe. The general rule is: All locking should use the same object.
|
|
|
|
|
Other Class may ONLY call OpenPort (or Similar ClosePort method - which has similar lock inside as OpenPort) method.
Otherwise indeed only tester uses that.
What you say now?
|
|
|
|
|
Member 12061600 wrote: What you say now?
Nothing else than I had before: Then it would be safe.
But:
Using it that way (a locking object in the Tester class) is bad style. It would not work anymore if you (or even worse anyone else) decide later to use it from within another class.
Again:
There is no need to lock upon opening (and closing). The exclusive behaviour of serial ports makes it unnecessary. If you want to open the port from different code parts, you must handle the execptions (they should be handled anyway). Using locking there does not prevent the exception. It may just delay it.
Conclusion:
Your current code is actually thread safe. But there are unnecessary operations, missing error checks upon opening, and it is prone to be not thread safe when changed.
|
|
|
|
|
"Using locking there does not prevent the exception. It may just delay it."
It prevents because if port is already opened I don't reopen it.
PS did you see the updated version of question? (with open and close calls). Thanks a lot!
PPS I understood your conclusion too, thanks.
|
|
|
|
|
Member 12061600 wrote: It prevents because if port is already opened I don't reopen it.
OK. You are right because you are checking the open state first. But it just returns without any error code or message.
But there may be other reasons that opening fails which should be handled (not existing, already opened by another application or another instance of your application).
So when handling the exception, there is no need for locking.
|
|
|
|
|
So we agree that although my approach maybe redundant at places
at least in its current form it should work (including the Open and Close methods being called like in updated question)
Thanks a lot for help
|
|
|
|
|
I have slightly updated my question with how open port and close port methods could be also called from other methods. Please give feedback on that if it is Okay.
|
|
|
|
|
See my above post. No need to lock when closing and (as noted in my initial answer) use SerialPort::IsOpen() instead of your own member variable.
|
|
|
|
|
I still prefer to have lock in OpenPort to avoid that exception you mention
|
|
|
|
|
Locking protects only against simultaneous calls. Because serial ports are exclusive, they are usually opened only once from within the main thread. When doing so, you would never have the situation of simultaneous calls from within your app. But you may have other open errors that should be handled.
|
|
|
|
|
To be honest, if I were doing this, I'd look at using a different architecture altogether because you're mixing a lot of responsibilities in your code here. What I would be tempted to go with is to have your API exposed so that you have separate writer and reader implementations, and the actual management of the port operation would be the responsibility of a completely separate class altogether. As I tend to compose objects together with IoC, I would constrain the port functionality to be container controlled so that all classes get the same instance and I would put the checking of the port along with its creation in there. I would also hide this as an internal implementation detail so you would have this as an internal interface with an internal implementation. That way, your client code only sees the separate reader and writer interfaces. Something like this internally:
internal interface IHaspPort
{
SerialPort Port { get; }
}
internal class HaspPort : IHaspPort
{
private readonly object SyncLock = new object();
private SerialPort port;
public SerialPort Port
{
get
{
if (port == null)
{
lock(SyncLock)
{
if (port == null)
{
}
}
}
return port;
}
}
}Then, you simply inject this instance into your reader and writer classes and let it take care of itself.
This space for rent
|
|
|
|
|
thanks but I am slightly beginner with .NET and I'd rather stick with my approach.
You can have a look I slightly updated question, if you have opinion - whether my scenario is OK now I'd appreciate. (I added open and close calls)
|
|
|
|
|
I thought about posting this question in the "Article Writing" forum, but decided it would get more "exposure" here, even if I risk this being seen as slightly off-topic for this forum.
The reason I ask is that I am thinking of doing an article for CodeProject that will demonstrate what explicit use of 'ApplicationContext can enable in terms of multiple window management, over-all application structure, and, more specifically, the all-too-common task of "communication" (passing data, raising/receiving event notifications, etc.) between Forms.
I've been using 'ApplicationContext (which is, as I think you know, being automatically instantiated/used "under-the-hood" of any default new Win Forms project) explicitly for years; couldn't live without it. But, I'm unclear as to how often this is used by other programmers.
And, is it, possibly, the case that at this late date in the life-time of Win Forms, such a topic has a small possible "audience" ?
thanks, Bill
«In art as in science there is no delight without the detail ... Let me repeat that unless these are thoroughly understood and remembered, all “general ideas” (so easily acquired, so profitably resold) must necessarily remain but worn passports allowing their bearers short cuts from one area of ignorance to another.» Vladimir Nabokov, commentary on translation of “Eugene Onegin.”
|
|
|
|
|
BillWoodruff wrote: And, is it, possibly, the case that at this late date in the life-time of Win Forms, such a topic has a small possible "audience" ?
Likely. Definitely smaller than the all the rage web technologies. But as I'm doing mainly WinForms as well I would be interested in reading it!
cheers, Sascha
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
Thanks, Sascha, if a "bulb as bright" as you are still working in WinForms, that encourages me to write the article.
cheers, Bill
«In art as in science there is no delight without the detail ... Let me repeat that unless these are thoroughly understood and remembered, all “general ideas” (so easily acquired, so profitably resold) must necessarily remain but worn passports allowing their bearers short cuts from one area of ignorance to another.» Vladimir Nabokov, commentary on translation of “Eugene Onegin.”
|
|
|
|
|
BillWoodruff wrote: a "bulb as bright" as you
Not sure how far this is justified but I'll take it and run 
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
In the excel sheet, I have more than 1 lac rows. Am trying to update specific cell and row. It's working and does the update only up to certain row but when I try A106880 row it's saying ' Make sure the object exists and that you spell its name and the path name correctly.'
string sConnection = string.Format(@"Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0};Extended Properties=""Excel 12.0 Xml;HDR=No;IMEX=1""", path);
OleDbConnection _Connection = new OleDbConnection(sConnection);
_Connection.Open();
string cmd = string.Format("Update [{0}$A106880:A106880] set F1 = 12", sheetname);
System.Data.OleDb.OleDbCommand myCommand = new System.Data.OleDb.OleDbCommand(cmd, connection);
if (myCommand != null) myCommand.ExecuteNonQuery();
_Connection.Close();
|
|
|
|
|
Please don't post your question in multiple forums. Choose one and stick to it.
Also posted in Q&A: Oledb excel update is not working[^]
If the brain were so simple we could understand it, we would be so simple we couldn't. — Lyall Watson
|
|
|
|
|
By mistake I didn't that. Do you have answer on this?
|
|
|
|
|
example (note: label1 and label2 can have only numbers):
If is label1.Text = 5 and label2.Text = 6
how to make a comparison, something like:
if (label1.Text < label2.Text)
{
MessageBox.Show("some text...",
this.Text, MessageBoxButtons.OK, MessageBoxIcon.Information);
return;
}
Please help. Thanks.
|
|
|
|
|
If you are trying to compare numeric values, then parse the strings into an appropriate number format first:
int l1, l2;
if (!(int.TryParse(label1.Text, out l1) && int.TryParse(label2.Text, out l2)))
{
return;
}
if (l1 < l2)
{
...
}If you don't, then string comparisons get a bit weird-seeming, because they base the entire comparison on the first different character pair then encounter in the two strings.
So the sort order isn't what you expect:
"1", "2", "3", ... "9", "10", "11", ... Instead it orders them as
"1", "10", "11", "12", ... "2", "20", "21", ...
Otherwise, just use the String.Compare Method (String, String, StringComparison) (System)[^] to return an integer indicating the result: 0 for "same", -1 for "first is less than second", 1 for "first is greater than second".
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
|
|
|
|
|
Thanks a lot, doing great. Problem solved.
|
|
|
|
|
You need to convert both strings to numbers first. The following simple test demonstrates that the string "82" is considered less than "9".
string s1 = "82";
string s2 = "9";
int foo = s1.CompareTo(s2);
Console.WriteLine("foo is {0}", foo);
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
