Rewrite the RX interrupt to remove goto and simplify debug statements

[edlongman]

The existing library was written using goto statements and in a way which masquerades as a State machine but it not really a state machine.

I have rewritten this interrupt as a state machine. See the diagram below:

No gotos now and it behaves much more like a state machine.

Also the UART debug statements have been compressed into a macro.

The structure that stores the bytes for received bytes has been made into a union so just the buffer can be written to as raw and the properties can be accessed separately.

//	+-------------trx_format--------------+
//	|             BUFFER              |sta|
//	|len|typ |chk  |                  |   |
//	|   |    |sum  |.... payload ...  |tus|
//	+-------------------------------------+

The Tx buffer struct could be constructed as struct of the rf_trx_buffer_t and the sync bytes. This would allow the txbuff state to be stored with the txbuffer

[tixiv]

Hi edlongman,

I am one of the original authors of this library, feels like a thousand years ago when we wrote this thing... Nice that there is still interest in it.
We used it at the lab at the time for some projects like a RF-joystick for our blinkenborg and I later also used it for a telemetry system for our solarcar. Sadly at the moment I don't have any test system ready to go to test your modifications.
I still remember it was not an easy task to make the lib work fine with all the features (small AVR-microcntrollers used at the time, some with only 2k of flash), and also the RFM12 gave us a hard time, as the chip itself behaves pretty strangeley in some cases, and not like the datasheet describes - I don't really remember the details.

I'm sorry, but I am not in favour of accepting this pull request, as I can't test it, even less with all the different configuration options. I only know the library has been working flawlessly since years in the devices that still use it. i would prefer to have this old project just left alone here.

Regarding a real statemachine: I'm not sure what that is supposed to be - evrything is a statemachine that has states and changes them. in my eyes, the old statemachine is very real (and tested, and working fine)

Regarding goto: I like that command, especially when needing to leave a pretty nested block to clean up and return or something similar.
It makes for cleaner code in a lot of cases. There was a paper at some point in the 70's "goto considered harmfull". Many people think the goto command itself is bad style nowadays because of that paper (not having actually read it). But that title is pretty "clickbaity" to use modern terms. A more suitable title might be "sphaghetti code considered harmfull", which then and now is pretty true. Goto alone doesn't make for bad style. Try grepping the Linux kernel source for "goto" for example...it's full of it, and not for legacy reasosns....

[edlongman]

Hi Peter, Thanks for the quick response. I understand you can't accept if you no longer have anything to test it with! If it's any comfort I have tested it on an AtMega644p and it worked perfectly. I can't say I've experienced any of the strange behaviour myself though through inspection with the oscilloscope and several hours of operation. Perhaps I am not using certain combinations of functions that cause problems. In a sense of a "real state machine" I should have said a Moore machine. Where the outputs are only set within a state. The intention was to have it more like how you would implement a state machine on an FPGA or ASIC. That is the rewritten version has next state logic based on the state before entering the interrupt and then state actions based off the next state logic. I would still argue in this case it creates spaghetti code and makes it very hard for a user to understand as it is not clear exactly what has been skipped and what has not been skipped. Best Regards, Edward

…

On Mon, 4 Nov 2019 at 18:32, Peter ***@***.***> wrote: Hi edlongman, I am one of the original authors of this library, feels like a thousand years ago when we wrote this thing... Nice that there is still interest in it. We used it at the lab at the time for some projects like a RF-joystick for our blinkenborg and I later also used it for a telemetry system for our solarcar. Sadly at the moment I don't have any test system ready to go to test your modifications. I still remember it was not an easy task to make the lib work fine with all the features (small AVR-microcntrollers used at the time, some with only 2k of flash), and also the RFM12 gave us a hard time, as the chip itself behaves pretty strangeley in some cases, and not like the datasheet describes - I don't really remember the details. I'm sorry, but I am not in favour of accepting this pull request, as I can't test it, even less with all the different configuration options. I only know the library has been working flawlessly since years in the devices that still use it. i would prefer to have this old project just left alone here. Regarding a *real* statemachine: I'm not sure what that is supposed to be - evrything is a statemachine that has states and changes them. in my eyes, the old statemachine is very real (and tested, and working fine) Regarding goto: I like that command, especially when needing to leave a pretty nested block to clean up and return or something similar. It makes for cleaner code in a lot of cases. There was a paper at some point in the 70's "goto considered harmfull". Many people think the goto command itself is bad style nowadays because of that paper (not having actually read it). But that title is pretty "clickbaity" to use modern terms. A more suitable title might be "sphaghetti code considered harmfull", which then and now is pretty true. Goto alone doesn't make for bad style. Try grepping the Linux kernel source for "goto" for example...it's full of it, and not for legacy reasosns.... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4?email_source=notifications&email_token=AAFJJUB2DDYJHB4YPJQK5IDQSBTCBA5CNFSM4JIVNDS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDAID3Y#issuecomment-549487087>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFJJUGN5YK4TRIN27ABCELQSBTCBANCNFSM4JIVNDSQ> .