-
Notifications
You must be signed in to change notification settings - Fork 34
Start of providing mock support for Wire.h #72
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
Conversation
This looks like a great start. The only thing I'm nervous about here is the use of PortDef
-- it's meant for representing the past and future states of an IO pin as strings. For something as rich as I2C, I could see that being used in one of two ways:
- Re-implement I2C, where the
PortDef
strings hold the raw data you'd see on the Arduino pin. This would mean that the strings are mostly incomprehensible to the user. - Use a big array of
PortDef
(7-bit, or 128 entries in size) that corresponds to each addressable device on the bus, so you can mock each I2C address individually. Might need to explore whether the mocked I2C devices should somehow implement DeviceUsingBytes - Make a new structure entirely to handle all the I2C mocking
In any case, I really appreciate that you've constructed the interface for this and I'm looking forward to getting this feature ready. What are your feelings about those 3 options?
3d8fd80
to
50f3fa3
Compare
The PortDef
variable was just an artefact from using the previous SPI support as a reference, and option 3 sounds correct. I have removed it, rebased and and pushed an update of the branch. I kept the removal as an additional commit, but if you want to it cost me nothing to combine the two commits into one.
BTW, I notice that you merge branches fast forward. I think it will be better to merge with non-fast forward because then you can see directly where features comes from, and also you do not need to keep old feature branches like 2019年01月06日_bugfixes
, etc.
Merging branches fast-forward seems to be a new feature; I didn't realize it was enabled and it's not my preference either. I'll see what I can do with this over the next few days.
50f3fa3
to
f019d72
Compare
Rebased to latest version on master. Also joined the commit Remove PortDef member for wire into the main commit Minimal Wire.h mock.
Thanks. I've been giving some priority to fixing recently-reported bugs in the library, but this is still on my radar.
f019d72
to
8f4b9af
Compare
Sorry for the delay, I'm ready to move this forward a bit more. Does any of the PinHistory
stuff I added as part of #116 make this mock easier to write? I'm still lacking a good understanding of how Wire.h
is normally used, which would be a big help for me understanding how to test a library that depends on it.
Thanks. I see that are now are some conflicts relative to the latest master branch, I will rebase and resolve.
Actually I have no use cases for testing I2C communication. In my project I am using the SmartLCD library which in turn includes Wire.h in order to communicate with the hardware device. But for all practical purposes I2C is just a serial interface dependency. I have no interest in testing the SmartLCD library and do not even want to depend on that particular LCD library so I have defined an abstract LCD interface class that I use everywhere in my code except for the actual wrapper class implementation. If I want to test something related to LCD printing I will mock that interface.
class LcdInterface { public: virtual void init() = 0; virtual void clear() = 0; virtual void print(const char *msg) = 0; virtual void set_row_col(int row, int coloumn) = 0; virtual ~LcdInterface(){}; };
Thus my use case is simply not having arduino_ci fail to compile just because Wire.h is included somewhere.
Most functions are empty and unfinished, so you will not (yet) be able to unit test I2C communication. But it will allow you to unit test code that is using libraries that uses I2C under the hood (e.g. LCD, RTC, etc).
8f4b9af
to
b91b9b8
Compare
Thus my use case is simply not having arduino_ci fail to compile just because Wire.h is included somewhere.
My apologies, I thought you were submitting this as a work in progress. I'll merge now and see what I can fill in.
Uh oh!
There was an error while loading. Please reload this page.
Feature Request
This pull request starts the work of providing mock support for
Wire.h
. It is modelled after the previous work adding mock support forSPI.h
. The mock functions do not do anything useful yet, but all the mocking skeleton framework should be in place, so the remaining part is "just" to implement some probes useful for unit testing. I do not have any use case for this, so I will not attempt to guess what would be useful here.But even without any under the hood functionality, this provides great value because it allows to unit test code that uses libraries that depends on
Wire.h
.Issues Fixed