HouseBot

While writing my HouseBot plug-ins, I decided to expand the plug-in support classes to be a bit more comprehensive and make writing future plug-ins easier.



In doing so, I found a few potential const violations, and I want to be sure I can depend on these particular pointers being returned to HouseBot to be honored as const (and if not, why). Really, I'm not trying to trash on the author of this code, but the pointers as they stand are interfering with desigining const correct classes being built on top of the interface.



in Devices/Common/DeviceAPI.h:

in struct DeviceInfo:

char *m_szDeviceInfo
char *m_szInterfaceSignatures
char *m_szModuleWizardDescription
char *m_szWizardFinalText


Shouldn't all of these be const? HouseBot isn't going to modify the strings returned via HBGetDeviceInfo is it?



in struct DeviceCallBackInfo:

HARDWARE_RC (*pHB_SendInterfacePackToInterface)(DEVICE_HANDLE, InterfaceArgumentPack *);


Is there any sane reason for the destination interface to modify the contents of the InterfaceArgumentPack?



in struct ModuleInfo:

char *m_szModuleVersion
char *m_szModuleName


same comments from DeviceInfo.



HBDeviceModuleInit( DeviceCallBackInfo*, ModuleInfo **ppModuleInfo );


shouldn't ModuleInfo ** be const ModuleInfo **? That's a pointer to a volatile pointer to const data, so it still works as an out parameter. DeviceCallBackInfo * should also be const since you do not intend for plug-ins to mess with this data.



HBGetDeviceInfo( int nDeviceNumber, DeviceInfo **ppDeviceInfo );


same comments from HBDeviceModuleInit for ppDeviceInfo.



HBDeviceSubscribeNotification( DEVICE_HANDLE, LPCTSTR szListName, LPCTSTR szFilter, CDataPack *pDataPack )


Same comments as the InterfaceArgumentPack from pHB_SendInterfacePackToInterface.



Also with the TCHAR pointers.. I see no follow-through using them as such in the example code and supporing classes from Common. Are there plans in the future to support unicode? I would probably use a more complete strategy for doing so, such as using char * and passing wchar_t * to entry points of a new name. TCHAR's are messy and really shouldn't be used on code that will never need to compile on non-unicode capable versions of windows. That's just my opinion.



in Interfaces/Common/HardwareAPI.h:

void (*pHB_NotifySubscribedDevices)( LPCTSTR szListName, LPCTSTR szFilter, CDataPack *);
BOOL (*pHB_RemoveIRCode)( IRCode* pCode );




Also a suggestion: If HouseBot allowed a user pointer to be stashed in a DEVICE_HANDLE, that might be more efficient than requiring the plug-in to store a map of DEVICE_HANDLE's to CDevice* instances.

Paul,



The APIs were constructed over time, so they do lack some consistency. I think most (or all) of your "const" changes are valid. I'll make the changes to the structs for the next SDK, but not to the callbacks. If I change it in the callbacks, it changes the function signature which then creates an incompatibility between old and new plugins.

Also a suggestion: If HouseBot allowed a user pointer to be stashed in a DEVICE_HANDLE, that might be more efficient than requiring the plug-in to store a map of DEVICE_HANDLE's to CDevice* instances.

That is a good idea. I don't know why I didn't do it initially.

ScottBot wrote:Paul,
The APIs were constructed over time, so they do lack some consistency.

I just want to make sure my plug-ins don't break HouseBot or vice versa. I'm not meaning to publicly critique your code :).

ScottBot wrote: I think most (or all) of your "const" changes are valid. I'll make the changes to the structs for the next SDK, but not to the callbacks. If I change it in the callbacks, it changes the function signature which then creates an incompatibility between old and new plugins.

I don't understand what you are saying about the callbacks. It's just a structure of pointers (function or otherwise), so the binary compatibility is unchanged. Or am I missing something that will cause my plug-ins to break?

Paul Forgey wrote:I don't understand what you are saying about the callbacks. It's just a structure of pointers (function or otherwise), so the binary compatibility is unchanged.

The one point you are missing is the fact that I get confused from time to time. I was thinking static not dynamic binding. I'll change them in the function defs as well.