|
static uint32_t usb_error = 0;
Don't care about what the compiler does. Variables that may be read before they are written should be always initialized.
usb_task_state = USB_DETACHED_SUBSTATE_INITIALIZE;
The variable will probably changed by the class. When creating another instance, the value is not the initial one.
bmHubPre = 0;
The variable is set in the constructor but the init function may be called multiple times.
Overall this is a bad example. When using global static variables with a class, you must ensure that only one instance of the class exists. To detect multiple instances some kind of check should be implemented. For your class this can be done by moving the re-initialization of usb_task_state to the destructor and checking the value in the constructor:
ASSERT(usb_task_state == USB_DETACHED_SUBSTATE_INITIALIZE);
Generally there is no need to use a class when that uses global static variables. When global variables are really necessary, they should be at least members of the class:
class USBHost
{
protected:
static uint32_t usb_error;
static uint32_t usb_task_state;
}
uint32_t USBHost::usb_error = 0;
uint32_t USBHost::usb_task_state = USB_DETACHED_SUBSTATE_INITIALIZE;
|
|
|
|
|
usb_task_state = USB_DETACHED_SUBSTATE_INITIALIZE;
usb_task state declares a new variable of type usb_task that is a member of the class.
|
|
|
|
|
But there is an underscore between usb_task and state.
|
|
|
|
|
I apologize; I missed the underscore. Never mind.
|
|
|
|
|
It doesn't look a good example.
Quote: static uint32_t usb_error = 0; // is (global) static initialized to 0 as default anyway?
Yes, however it doesn't harm.
The other pieces of code are ugly and your observations right.
|
|
|
|
|
Like others mentioned this isn't the cleanest example, but I'll also take a stab at explaining some of these things...
static uint32_t usb_error = 0; static uint32_t usb_task_state = USB_DETACHED_SUBSTATE_INITIALIZE;
Initializing a global (or a static) outside of the user function or class is always good practice. The compiler may or may not initialize for you, but why risk it.
USBHost::USBHost() : bmHubPre(0) {
usb_task_state = USB_DETACHED_SUBSTATE_INITIALIZE;
init();
}
This is initializing a global that is in an undefined state back to the default. Remember that if there was an instance of USBHost that existed, he might have left the state of the variable in some unknown state. Like someone else mentioned however, this initialization does imply you should only ever have one instance of this class... otherwise you'll have a conflict with the state of this variable. With that said though, it is resetting itself, so you should be able to delete the object when not in use and make a new one when you do need it.
void USBHost::init()
{
devConfigIndex = 0;
bmHubPre = 0;
}
init() methods are usually made when there may be a need to initialize variables outside of the constructor, or alternatively, when there are multiple constructors that all need to initialize the same variables. For example, there may be some code that gets called when the USB device gets to a certain state that requires re-initialization.... rather than destruct the object and start all over, the init() may be called to re-initialize.
|
|
|
|
|
Thanks guys, I really appreciate your help.
This class is to make USB host (Arduino) communicate with USB devices and so far I'll be happy if I can work with one device / one instance of the class. Eventually I'll need a USB hub (as USB device) to work with multiple devices so I'll make sure multiple instances of this class can exist.
I sure wish there was an easier way to "see" the objects hierarchy in Arduino code.
But I feel I am really pushing my luck with this toy hardware.
Thanks
Vaclav
|
|
|
|
|
Vaclav_Sal wrote: I sure wish there was an easier way to "see" the objects hierarchy in Arduino code.
I'm not sure where you got the code from, but there's auto-documentation engines such as Doxygen[^] that should make object hierarchies easier to see. A lot of open source projects use this type of "auto-documentation".
|
|
|
|
|
Thanks, this especially may be what I am looking for.
"Doxygen can also visualize the relations between the various elements by means of include dependency graphs, inheritance diagrams, and collaboration diagrams, which are all generated automatically."
|
|
|
|
|
|
Albert,
this doxygen is a FANTASTIC tool!
Thanks a milion!
Vaclav
|
|
|
|
|
Quote: The compiler may or may not initialize for you, but why risk it.
That is not exactly true. The C++ standard specify that such variables 'shall be zero-initialized', hence no risk, unless the compiler is not compliant.
|
|
|
|
|
CPallini wrote: The C++ standard specify that such variables 'shall be zero-initialized'
Was this in every C++ standard since the original? ...I don't really know...but if it wasn't, it's not worth any savings in writing. Plus it makes it easier to read.
|
|
|
|
|
Albert Holguin wrote: Was this in every C++ standard since the original? ...I don't really know...
I think so (it comes from C).
Albert Holguin wrote: it's not worth any savings in writing. Plus it makes it easier to read.
I completely agree on that.
|
|
|
|
|
I was just having a discussion on initializing variables with a coworker. We're working with python a lot and the absence of declarations is driving me mad. Python is simple and fast to write but you can definitely make some very ugly code with it (large classes with multiple inheritance within the same python file, ugh).
|
|
|
|
|
|
FYI
After very brief usage of doxygen I have found that I indeed have several USBHost classes initialized hence I need to clean up current definition - especially the static global "members".
Appreciate all your help.
Vaclav
|
|
|
|
|
Glad you found useful info. Anyway I still think that code is ugly.
|
|
|
|
|
I am sorry to reopen this, but I just need confirmation that I am on the right track in reorganizing the code.
I only hope I can describe my plan.
Basically I have 3 USB classes. I hesitate to call them “objects “ for now.
To bypass Arduino “help” ( trampling on my code ) I have this flow.
It may be overkill but it is working so far.
C_Main
Cprocess
USB (device) - “interface “ class member of Cprocess (my code)
(device)Controller – controls the USB device – mouse is working (mostly cut and paste)
USBHost - a native access to Arduino Due USB port for now used / tested as a state machine to retrieve USB configuration from attached USB device ( works so so )(also mostly cut and paste from ATmel)
I would like to have all of these USB(device), (device) Controller, and USBHost in some kind of hierarchy. As you pointed out to me , the “problem” is that USBHost class itself is “global” and some of the class parameters – for example state of the “state machine” are also global.
I can fix that.
Eventually I need to control all of the devices on the USB (bus) and would like your opinion if the above scheme is OK as a base.
Many thanks for your time , appreciate it very much
Vaclav
|
|
|
|
|
Some basics:
Is-a = inheritance. If you inherit type B from A, then you can use type B as type A. Also see Liskov Substitution Principle (LSP).
Example:
class Shape { virtual void Draw() = 0; };
class Circle : public Shape { void Draw(); };
A Circle is-a Shape, so type Circle inherits type Shape. Note that this is all about behavior (interface), so you can treat a pointer or reference to any subclass of Shape as if it was a Shape with respect to Shape's interface (in this case a single Draw() function).
Has-a = aggregating.
Example:
class Canvas {
public: void Draw();
private: set<Shape*> m_shapes;
};
A Canvas can hold one or more Shape's, of any concrete subtype, and most likely Canvas::Draw will iterate over the shapes it holds, calling their Draw method. For clarity, you may want to think of Canvas::Draw as Canvas::DrawShapes.
For the particular case where you are to have one, and only one, object of a specific type, you may want to have a look at making it a Singleton.
I hope this attempt at a simple explanation of the concepts can help you come up with a design. While drawing has nothing at all to do with USB, the concepts of inheritance and aggregation are the same. For project after project.
(NOTE: code-formatting was done for compactness, please do not copy as-is  )
|
|
|
|
|
Mike,
thanks for reply.
I just found another copy of the USBHost class and in the few comments attached to it they use term singleton. So your comment arrived just in time.
I really need to dig into this because the sample code I have been playing with does not work when more than one USB devices are on the USB bus.
I think I got the "static class" concept but still struggling with how each inherited class uses global state variables - since each USB device maintains his own state machine.
Thanks
Vaclav
|
|
|
|
|
To me, just based on this comment, it seems you'd have:
A "base", the driver. The singleton. This owns one or more
USBBus objects, each handling a physical connection. These in turn owns zero or more
USBDevice's, each representing a device on that bus.
Each USBDevice would in turn probably have one or more "functions" (e.g. a USB mouse with buttons, wheel, and some more clickable buttons could easily present 3 or more functions/sub-devices).
It also seems logical that each device (and function and so on) might need (hold) a reference to its owner, to f.ex. be able to notify it about events. Not to mention the obvious - verify correctness in such a highly dynamic and unpredictable system
I see no obvious reason any of the USB types should ever touch the global/static parts. But should the need arise; don't give them access to the data (if you ever used MFC, consider that the anti-thesis of good design). Instead provide them an interface - and not only an interface that returns a ref or ptr to the global/static var's, but something that actually do some work.
Which brings me to another good rule-of-thumb (actually, this one is more like a law of nature):
- Interfaces shall be minimal and complete.
Additionally:
- Member functions should do two things; Use the object's state (directly or indirectly), and not leak the objects state to the outside. If a function is not using the object's state, it has no business being a member function (not counting corner cases). If it's not hiding the internal state, it's "leaking" it to other types to use or abuse, and making it hard-to-impossible to change the implementation later on.
That last point, not leaking internal state to the outside, is worth hammering in over and over until you say "Obviously, anything else would be insanity!".
Later, if needed, "on the side" you might even find it useful to have a "registry" of devices, no matter what bus they are on, to save lookup time or whatever. F.ex. the system input handler might poll you about input events, and then you could have one list of devices dedicated to HID input. Another case could be the user requests "Put all storage devices to sleep" and instead of walking the busses and their devices (which in turn may have an expander (hub-ish) with another bus to dive into) you have a single list to iterate. You get the idea.
++luck;
|
|
|
|
|
I am receiving Binary data of an image in 4K chunks through USB. I need to get this data and display the complete image(as GIF) using VC++. Please show me the way(examples preferred).
|
|
|
|
|
What are you receiving exactly?
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
