Closed heliosfa closed 4 years ago
Some XMLs that have been used for testing:
Worth noting that this changeset effectively removes P_Sup_Msg_t, and replaces it with P_Msg_t. Could you explain this change in more detail? I get that you're unifying the packet format for supervisor-bound and normaldevice-bound packets, but I don't fully understand the ramifications of this change sweet_potato.
Not much to explain other than all messages use the same size and layout of header, though some fields are usurped for other uses in certain cases.
The big changes were that a lot of irrelevant/duplicate information was removed from the start of the payload (e.g. the hardware address, and the sender address)
Having a constant message size makes everything easier from all perspectives - the user doesn't have to "manage" with smaller messages to supervisors or externals, we can decode messages easier when debugging, etc. etc.
I note that P_Super_Msg_t remains defined in poets_msg.hL163 (see black_flag), and I'm confused as to why
Because I am an idiot and forgot to remove it.
Because the Supervisor needs a way of keeping the hardware address with the packet to be sent.
Have you given a thought as to how we might implement debugging over UART? It would be cool to have "ready configuration" and "shutdown" reports back to the Mothership (as per S7.2 of your documentation)for debugging purposes.
UART is one of the things I ignored. I, at some point, need to go back to the trivial log handler and make it dump over UART instead of blocking for network.
But in the general sense, having a UART Debug "API" could be good, though network packets should generally be preferred as they are faster and (should) block for less time.
Documentation feedback:
* Casing of "Softswitch" is inconsistent - I'd go with "Softswitch" throughout.
Done
* I recommend a verbatim style for structures and constants in code, e.g. `ThreadContext` and `0xEF`, to aid readability. I see you've done it in some places!
Will do
* ADB may disagree here, but I would prefer separate `P_builder` (can we come up with a non-code name for this? How about "Source Builder"? smile) and Softswitch documents, with each document detailing the input/output between the two systems. `P_builder` is more than just the Softswitch - it also deals with the Supervisor, and people looking for the `P_builder`--Supervisor relationship should not need to read the Softswitch document
Agreed, this is where a lot of things get into a tangled mess, and separating the builder from the softswitch is a good idea.
* Data structure figure and tables were useful when reviewing the source; thanks for those!
:+1:
* Can we add configuration-file-style options in the future work section (regarding S2.1)? "Table X" is also an interesting label wink.
Can do. I take it that you don't like Roman numerals ;)
* Your "Main Loop" section (S3.2) is very good, and succintly describes Softswitch operation, and has saved me time chasing the control flow around banana. Figure 4 is also useful, and is the key figure of this document (if you're only going to look at one thing, look at that!)
:+1:
* The opening of your "Message Format" section (S4) seems a little redundant with the software address documentation. Given that these both match with each other, it is worth referencing the software address documentation for people that want more information on that topic. To be clear, I'm not against the redundancy, as context is required to set up the discussion around Opcodes (S4.2), and to meaningfully explain what the pin address is used for in (S4.3). sweet_potato.
There is a placeholder XXXXXXXX in 4.1 for a link/referenece to the software address docs - I just have not got around to populating it.
* Regarding "Log Messages" (S5), can you add some examples of fragmented log messages? Explicitly stating this would help when I build the other end of this pipeline (Mothership).
Can do.
* $240000000$ -> $2.4\times10^8$. (S6)
This is Word, we don't have fancy Latex formatting ;)
* Regarding future work, what is the motivation behind S7.1? Faster access for the thread context, yes, but that presumably means something else cannot use SRAM (no free lunch). What's the tradeoff?
Nothing currently uses the SRAM because we don't let it. We could allow applications to use it, but this will not be easy in the general case (also there is no concept of different speed memory in the XML) and will likely add un-needed complexity. In other words, currently, there is a lunch that has already been paid for and I want to eat it rather than letting it go to waste.
The suggestion of putting thread context at the base of the SRAM heap was just to get it out of DRAM and to make some use of it. The SRAM heap is probably also large enough to put the rtsBuffer in as we have now constrained the size.
* Again regarding future work, have you given any thought as to how we could support different Softswitches in future (i.e. variants of Figure 4 for different applications)?
This is briefly touched on in 7.8. Compile-time defines (sourced from a config file or environment) are the direction I would head).
Well, it's done now!
This pull request is for the much anticipated softswitch refactor. Essentially, it is a refactor/simplification of the existing softswitch, implementation of the agreed upon software addresses and the required alterations to P_builder.
Initial draft documentation (docx | pdf) is available. This will be converted to Pandoc and added to the docs repository in due course.
Much testing has been done, and this works with heated plates up to sizes that fill a single box. This has been tested with the new netstress example and one of @AlexRast's synfire XMLs.
This also passes the Orchestrator's included automated tests.
The equivalent branch in the Orchestrator_examples repository will also be merged into development when this is accepted.