MX682X / Rewritten_Wire_for_DxCore

A rewritten Wire library for the DxCore and megaTinyCore
0 stars 1 forks source link

Updates #6

Open SpenceKonde opened 2 years ago

SpenceKonde commented 2 years ago

(using this because no other way to contact you >.>)

Good news! There's a register_model demo (it's not that great, but I think it shows the general concept. I implemented it as getBytesRead(); returns how many since the last time it was called.

I also added a slaveTransactionOpen(); that returns non-zero values for read in progress (poll it until it's not) or write in progress (there a write in progress but the master isn't done yet)

I think the last remaining thing before I feel like I can declare new wire "done" is merging the slave mode buffers, since as we established, that is safe. But I'm a little nervous poking around in there for that. Do you think you'll have a chance to look into this, or at least let me know what the obvious pitfalls are here?

The other day I added the abbreviation for wire modes so that export_compiled_binary will have "WO, WA, WO2, or WA2 in the name (for Wire master Or slave, Wire master And slave, Wire master Or slave 2 modules, Wire master And slave 2 modules, and also renamed the define from USING_WIRE1 to TWI_USING_WIRE1 - so both the TWI defines look like they belong together. ("Any tool menu option which can impact binary output shalt have an indication in the name of exported assets. Oh, and there's now a script in my AVR-Research repo that does two things: Demonstrates that I'm shit at python, and "fixes" the horrendous formatting of the memory maps we generate.

Serial in event mode needs to be tested as I'm sure it will pose problems,

And I've got some loose ends w/event library that need to be tied up and random little things, But soon I think I'm going to make a board manager release.... on a new pre-release json file. and do that for megaTinyCore and DxCore so people can more easily start finding bugs

SpenceKonde commented 2 years ago

What was that part that couldn't use master and slave mode, and why? Need to cover that in the docs, and the menu entries indicate that both master or slave and master and slave mode are unsupported on tinyAVR 402 and 202, and but you also had a master and slave size test on the 2k one I thought.... so I'm trying to understand that

MX682X commented 2 years ago

What was that part that couldn't use master and slave mode, and why?

Only the 202 and the 402 do not support simultanious operations, according to the datasheets. The 212 supports it. I guess it had something to do with the die.

I've scrolled a bit through the documentation and it would be great if you added following to the swapModule: It can only be used when there is no Wire1 object. (When selecting the mode in the board menu)

About the implementation of a merged slave buffer: Where do you have your updated code? I can implement it somewhen in the next week, but it would be easier to work directly on the newer version. But, to make the code clearer: There are basically 4 cases that were done with #ifs:

there are basically no pitfalls. And the way I did the assignment, only the beginning of the relevant (slave) functions has to be changed. The code is basically for the compiler to tell that this local variable name is actualy that one in the global space. Only thing one might mess up is the naming of the variables, there is _tx, _rx and _tr (Tx and Rx merged)

P.S.:

(using this because no other way to contact you >.>)

Yeah, at the beginning of the year I've never thought I'd be still involved... (Don't get me wrong, it's alright, just unplanned))

SpenceKonde commented 2 years ago

https://github.com/SpenceKonde/DxCore/tree/master/megaavr/libraries/Wire has the latest version.

There's only a small to-do list before I do DxCore beta board manager release (under different json

I threw together a register model example. It's super crude, but shows the concept and how getBytesRead works - I think even just showing it to be possible now, and making sure that the concept of how onRequest works - that's what the big payoff is from. Someone making an I2C slave, I think, will know how to take it from there.

MX682X commented 2 years ago

When rewriting the code, I had the thought: "Wouldn't it be useful to be able to access the slave buffer part from the main sketch for debugging?" Basically, right now, it is basically impossible to check the slave stuff from outside the interrupts except with a debugger, so maybe it would be useful to add one or two functions that set the bool that that changes the access from master to slave and back (it's _toggleStreamFn in the struct twiDataBools). And give the instruction in the documentation on how to use it: cli(); slaveStreamFnc(true); yourCodeHere(); slaveStreamFnc(false); sei(); Would it be useful to implement that?

SpenceKonde commented 2 years ago

Would that be as simple to implement as it sounds?

I imagine it could be useful.... but i think the number of people who will use it is probably a small subset of the people making I2C slaves, who are a small subset of the people using these libraries. Any idea what the overhead would be like for people who are not using it? I don't really understand what the logic is behind what the compiler can and can't optimize away for classes. It seems that classes are kyrptonite for the optimizer.

If the overhead is nil, and it's easy to implement it, go for it.

If the overhead is enough that someone on a 4k part will notice it (ie, unless it is very close to nil, I would hesitate (though if you think it's really useful, it could be made conditional on flash size, like turn it off below 16k or something). I'm not sure how truly useful having access to the slave buffer from the rest of the sketch is, or rather, how often someone will be in a situation where that is useful.

I wonder if instead the debugging feature to access that buffer should consist of just having a way to get a volatile const uint8_t* pointing at it. Then you could even do stuff like polling it as bytes came in from the master. And since bytes are atomic you've never see a gibberish value there. Does that make any sense, or am I misunderstanding the purpose?

BTW - the cores now both have all the serial ISRs done in assembly.

Saves over 1k of flash if all 6 USARTs are in use on a Dx 64 (though note that this was partly to try to get rid of some of the overhead that was added by my adding a TXC interrupt that is used only in half-duplex mode.... I wish there were ways to give the compiler hints about when it can drop certain things.... I hate how classes are like kryptonite for the optimizer.