rogerclarkmelbourne / Arduino_STM32

Arduino STM32. Hardware files to support STM32 boards, on Arduino IDE 1.8.x including LeafLabs Maple and other generic STM32F103 boards
Other
2.52k stars 1.26k forks source link

F1 Wire - remove master transfer function parameter #831

Closed stevstrong closed 3 years ago

stevstrong commented 3 years ago

The i2c_master_xfer() function has the parameter num which refers to the number of messages passed by the pointer msgs https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/b6eb20d00a4010d83b386b2cff830a363cfebc34/STM32F1/cores/maple/libmaple/i2c.c#L349-L368 wherein the caller functions always use the value 1. I see no point to have more than one message to transfer over I2C at a time. Thus the code can be simplified and it would be more readable.

dewhisna commented 3 years ago

These low-level functions are used by MORE than just the horrid Arduino API. While it's true the Arduino API only sends one message at a time, I have applications using the intermediate level functions and need this functionality -- part of why I rewrote that part of the code.

If you are going to go change these, I'm going to have no choice but to take my fork and branch out on my own with this and diverge from your master branch...

stevstrong commented 3 years ago

I woudn't mind to rethink the issue if you could show me one application which uses this to see the reason of this parameter.

Anyway, I thought you are using the Wire_save lib, not the Wire which I actually targeted. Don't you have this functionality in Wire_slave?

dewhisna commented 3 years ago

I am using WireSlave in some places, but I also have applications that don't use either (and doesn't really use the Arduino API at all) and instead uses the core functions directly. Presently, WireSlave is also Arduino API conformant. New functions could be added to that TwoWire class that were extensions to the API, but I didn't do that because I assumed you wanted the main API of this framework to be the Arduino API.

dewhisna commented 3 years ago

And if you are wondering about the use-case, it's used on things like OLED displays where you are sending lots of back-to-back messages to draw things on the screen. With this, you can queue up a stack of messages and the I2C core and IRQ code will automatically use the restart mode with the device to send the additional messages and does so without further application intervention.

stevstrong commented 3 years ago

Ok, if you are using the low-lewel core functions for sending several consecutive messages then you could actually call consecutively the same function for each message in part, right? But I see that in this case you need further application intervention so you lose a certain degree of automatism.

dewhisna commented 3 years ago

Right, you can send them consecutively, but then your app code has to periodically check to see if it can send the next message and there's lots of extra overhead there. Ideally with something like a display, the app needs to be able to update what it wants to display and move on with other things and not be concerned about getting it to the display device.

I'm mainly using only the core of this repo and very little of the Arduino API parts. This core is a better alternative to the code in the STM32Cube stuff, which looks like it was written by a hardware person who perhaps knows Verilog and VHDL (i.e. languages that are synthesizable to hardware) but knows next to nothing about writing software (sort of like whomever designed the blasted I2C peripheral on this processor and made it so horrible to deal with in software).

I started writing my own core originally for the F4 and then extended it to the F7 and have been adding some F1 things for other projects that needed lesser hardware, but I ran out of time and needed to pull in a few working pieces to get on with the program, hence my use of this repo.

I was just thinking, though, that in any case, it's going to be difficult to keep a stable API to that core layer, as that's not your goal on this repo -- but rather having a stable Arduino API implementation. So, maybe my best bet is to let our repos diverge. I'm sure there's going to be more changes I'll want to make to the core that you'll not want to pick up and vice versa.

stevstrong commented 3 years ago

Ok, now that I understand aproximately what it does, I don't want to change a code which has some features, but it would be nice to have an example how to use it. Can you eventually post here one, or push as a PR?

dewhisna commented 3 years ago

You probably don't want to mix Arduino API and Core API examples and if you start making the Core API "public", you'll need to document it and make it stable. Using this isn't quite as simple as a different function call or two. You essentially have to write your own complete replacement for the TwoWire class and integrate that functionality with the rest of your application.

I'm using a variation of the old .NET framework that targeted the STM32 processors, such as was used on the old NetduinoPlus2 boards. The I2C implementation there is completely 100% asynchronous to the application (as opposed to the blocking calls of the Arduino API) and is designed for use with code continuations. Now, I'm not using C# with it, only some of the C# concepts. Continuations can be somewhat modeled in C++ using lambdas.

Writing a trivial example isn't possible. But the essence involves allocating an array to hold messages, allocating the data for each of those messages, and calling the i2c_master_xfer function with that array and array count. That code would need to be hooked into the receive and transmission completion events so that it can free the memory and signal the main app of the status. Writing a trivial example that just did 3 or 4 messages in local variables and calling that function isn't a very good example because it's probably just as efficient to send them one at a time and let TwoWire handle the buffer allocations for you.

What I have going in my app is the continuations logic somewhat derived from https://github.com/netduino/netmf/tree/master/DeviceCode/pal/AsyncProcCall and the I2C logic that plugs into that: https://github.com/netduino/netmf/tree/master/DeviceCode/pal/COM/i2c. That logic takes the place of TwoWire in the Arduino world. But, for the back-end, instead of using their core: https://github.com/netduino/netmf/tree/master/DeviceCode/Targets/Native/STM32/DeviceCode or the STM32Cube based core, I'm using this libmaple core, which has been less convoluted and worked well to get things up and going quickly (even if I did have to rewrite the I2C piece).

So I really don't think you'd want an example here like I have in my applications. Though what would make sense for this would be to extend TwoWire to add support for more than one message -- something that implements a bit of the logic similar to the first two links above, but scaled back to be more I2C centric and less complex.

stevstrong commented 3 years ago

Closing this as it offers a nice feature which could be exploited when needed.