Hi, > First, it is kind of awkward that I'm working with a "random" branch > of xpcc, in that I don't know where to apply bugfixes (I found a few > minor ones) so that they get back to my branch. Should I fork xpcc and > create a new "working" branch from master, and merge in the stm32f103 > support branch and then later the new commits from master, and publish > the bug fixes on xpcc:master? (so if a new commit arrives to master, I > can merge it back to my branch)
The 103 branch that you are on is only required for SystemClock and linkerscript support, it doesn't do anything else. It builds on feature/system_clock which will probably not be merged in the near future since a lot more research into clock tree abtraction needs to be done. We should backport the required specific system clock tree for the F103 though and merge the support into develop (not master!). Our master branch is old, I'm not even sure why we still have it. We are lacking a release strategy. Your F103 branch is up to date with develop, so you should commit your bugfixes on top of it and then cherry-pick them to a seperate feature/bugfix branch on top of develop. Then make a Pull Request for each one of them for Code Review. So yes, you need to fork. To be clear, I don't intend to merge the F103 branch into develop, since it would mean merging the unfinished feature/system_clock branch as well. Unfortunately the system clock thing is a lot more complicated than I thought, so sorry for the complicated cherry picking approach. > Second, I noticed that on the I2cTransaction class, which is supposed > to be a general abstract class of every transaction, there are > "configureWriteRead", "configureWrite" and "configureRead", which > doesn't make sense to me (they aren't even implemented). Also, of the > 3 classes which implement these all have all 3 methods even when it > doesn't make any sense (like "configureRead" on > "I2cWriteTransaction") Ah, you arrived in the belly of the beast. The I2cMaster delegates I2C state machine decisions to the I2cTransaction via the virtual callback methods. The `configure*` methods should be pure virtual too, as they are required by I2cDevice in its `start*` methods. However, since the class type is passed by template anyway, these methods do not need to be virtual, because the derived class type is known and therefore used _directly_. The methods only exist in the base class for documentation. Since I2cDevice is the same for every I2CTransaction type, it uses the methods `configureRead` to provide `startRead` even if it makes no sense for this device. Thats why they are protected, the driver inheriting from this class needs to abstract this away anyway. Have a look at the concept description for I2C for the state machine graph (ignore the SPI stuff, we do that differently now): github.com/roboterclubaachen/xpcc-papers (there is a rendered PDF under concurrency-modelling/document, page 11 has the graph, page 6 describes transaction objects). > Now, I figured I would write a transaction that would be > register-oriented (first byte of data is register number, then read or > write). On this transaction, the aforementioned methods should simply > not exist, but that's currently impossible. The I2cTransaction describes the I2C state machine, but shouldn't know anything else about the semantics of what you are doing. So, you _could_ do the register abstraction directly in a custom I2cTransaction, but I _really don't recommend_ it. The only two devices that use a custom I2cTransaction are the SSD1306 OLED display (multiple bus transfers in one transaction) and a generic eeprom driver. The latter requires a start-write(16bit address)-write(data)-stop, on non-continous memory so that the storage data does not have to be copied behind the 16bit address. So here a custom I2c state machine was required, but the use of these special transactions are very rare. The other I2c drivers already use the register-oriented concept and expand on it by abstracting away the transport layer altogether. Please use the WriteReadTransaction and build on top of that. I recommend looking at drivers like the ITG3200 gyro, HMC58x3 magnetometer which use this transaction and register abstraction extensively. (and perhaps Lsm303a accelerometer for transport layer abstraction). There is an entire blog post on register abstraction on blog.xpcc.io. > Third, I wonder, why is "draw" a function pointer in "GraphicDisplay" > and not a virtual function? I couldn't yet figure it out... `draw` points to either setPixel or clearPixel (both virtual), so that the actual drawing method (for example Bresenham line interpolation) only has to choose the pixel and not worry about what to do with it. Please note, that the entire graphics subsystem was written for small binary displays driven by AVRs, and color and GUI was sort of added as an afterthought when we needed it. I'm working on an entirely new graphic system for Cortex-M over here: github.com/salkinium/pain-ter. Keep on hacking, you're very attentive and we need people like you for xpcc! Happy New Year, Niklas _______________________________________________ xpcc-dev mailing list xpcc-dev@lists.rwth-aachen.de http://mailman.rwth-aachen.de/mailman/listinfo/xpcc-dev
