electricui/electricui-embedded

Add communications functionality to connect your hardware to a local user interface.

electricui-embedded's Issues

Review options for 2D char arrays

We've hit into the issue with character arrays being hard to support again. To record the current understanding/discussion:

"We'll just use a custom delimiter which will let us know the start and ends of the legitimate strings"

This has issues because (as far as I know) you can't get the size of each string in C without the developer defining it, which would require information in the metadata we don't currently support.

Multiple objects from the same variable

While the developer could internally use a 2D char array (array of strings), they would need to break each of them into a eUI object with base address of each element. (then the macros will require additions to allow for smaller sized payload lengths).

I did actually do this with the Delta project, the array of structures is supplied to the UI as 4x single structures.

Allow the UI to determine the size of each string

One way of handling this is to allow the developer to specify the maximum length of the 'string' array dimension, then write the whole thing out and let the UI catch the null termination?

Allow custom types?

Perhaps the best way to handle this is to provide some examples for the developer to create a custom type for their strings, which contains the information required in each element?

Handling non-C strings

C++ and other higher level platforms don't have the same base issue as the C implementation, they should be able to see the size of the x-dimension, or have a native 'string' type which embeds the necessary data.

---

This needs more research, thinking, and probably some trial implementations.

C++ compiler has issues with developer scoped functions being passed as void* to the eUI object array

We don't have issues with this for the internal variables, as they are inside the extern { }, but developer space callbacks are an issue.

I need to read up on the correct interoperable syntax for function callbacks (if it exists, a quick search was inconclusive), and then implement it, or provide an alternative.

STM32 testbed experiences hardfaults while trying to report an error 4

STM32 gets into a position where it hardfaults due to eUI (not great).

Stacktrace doesn't report responsible line, but working backwards:

• Packet encoding was in process.
• The intended output was a 'er' message with code 4 (invalid developer variable which failed search).
• The message in question had a seemingly valid msgID with valid 0 for internal bit in the header.

Add support for encryption of binary data between SOH and ETX

Allow the developer to provide a key or value which is used for some light crypto (optionally), to provide encrypted transport.

This would involve a minor refactor of message generation and ingest functions to support a compile time flag to enable encryption of messages.

Fill out the expected bytes for handshake process

Unit tests have been written for the innternal functions, but rely on doing byte-level comparisons against a ground truth.

We need to work out what such a valid series of bytes is, and potential variations to these messages which might be considered erraneous values.

Fix 32-bit architecture support

It doesn't work at the moment.

Rename sequence numbers to ack numbers

This makes more logical sense.

• Do it.

Don't call callbacks on queries

As per #27, we might not want to do that.

Investigate why large arrays often have a 'long' period between packets

Probably due to the serial buffer being filled too quickly and some kind of flush behaviour or waiting time takes longer to complete the transaction.

Seems repeatable between test programs, helloboard1 and ESP32.

Need to debug further on different platforms.

Introduce concept of read-only variables

Background

For many uses, the microcontroller may not want to provide write access to a given variable.

This could be values stored in ROM, non-editable values like calibration registers, or values which shouldn't be modified by the UI but managing a copied array is heavier than needed (situations where DMA is writing sensor values into a ring buffer at a high rate, etc).

The eUIMessage object structure currently uses a uint8 for the type metadata, but we only use a few bits of that byte.

Proposal and impact

I propose using one of those spare 'type' bits to specify a writable flag.

• This means there should be no hit to memory or ram in the structure storage.
• Checks can be done with bitwise maths at low computational cost.
• The RO state is enforced at the microcontroller side, rather than 'asking nicely'.
• We can use macros to make this easy for the developer.
• The UI handshake process should distinguish the read-only variables

Affected behaviours

• We don't have a spare bit in the messages to indicate this on a per-packet basis, and we wouldn't want to.
• Its going to be enforced at compile time, so its immutable anyway.
• The handshaking process needs to give the UI an indication of writable values in some way.
• We can probably resolve this by splitting the message transfer process to the micro, where writable and read-only variables are split into two groups.
• The overhead from two sets is one additional message, so the overall bandwidth cost is approximately one message (13 bytes?).

The user interface then needs to do something useful with this information, but can be addressed later and in the correct issues trackers.

UUID set function still has placeholder fallback value of 4

This needs to be revisited, along with the concept of human readable strings for users to 'name and identify' devices.

https://github.com/Scottapotamas/electricui-embedded/blob/d5619c985ec3ec051804e76a98ad3bd90cb14311/electricui.c#L281

Trust the packet, or internal storage's message type

When handling a parsed packet, we check if the type is a callback before writing it to memory (as callback payloads are the pointer to the function on the micro).

This means that callbacks are a custom 'event' message, and can't be used for both data transfer and the function call.

In favour of trusting the UI

In theory, if the UI was allowed to modify the type of the message, it would be capable of keeping track of the pointer and variable, and send two messages with mutated types.

In favour of trusting the micro

Allowing the UI to specify jump locations is a Bad Idea™ because it opens a security vulnerability where a malicious UI can request jumps to different memory locations.

Additionally, writing over the variable on packet ingest with current code would require a different declaration technique, as the inbound "not a pointer" variable would overwrite the pointer. This would introduce edge cases which require more handling code.

Decision

Therefore, the microcontroller will trust the internal stored pointer value for a callback flagged message, and as such, the UI has room to remove a few bytes of traffic if needed.

Work out how to handle read-only developer variables

The first example is 'const' or 'static' variables, but potentially the developer doesn't want eUI writing specific variables?

Get wifi working on the esp32

ESP32 wifi support with the embedded library is becoming a critical path item for mobile support.

Maintain 'connections' to support multiple comms methods

The parser is stateless and processes a character at a time (at current).

Propose holding the parser information in a structure, then passing in the structure by reference when inserting new data.

This would allow a "USB" and a "RF link" copy of the state machine, and when data is recieved from either, they can process messages without stepping on each others toes.

Drawbacks: holding a buffer for each method, plus a few ints.
Benefits: Supports multiple simultaneous message inputs.

From a usability standpoint, it might be better to put these structures in an array and pass the index around to allow for enum style use (this is a usability change I'd do later)

CRC algorithm doesn't pass some tests

While writing unit tests, found some fun behaviour (which should probably be in @Mike-Dax CRC unit tests too).

We init the CRC with a 0xFFFF, as CRC-16/CCITT-FALSE.
Send in 0xFFFF and we should get 0x0000 back.
Any subsequent 0x00 bytes still returns CRC of 0x0000.

This self-negation property should pop up for other values, so I'll continue adding tests.

We need to look at different seed values or mitigation techniques, or just accept the risk.

Internal messages should use an int/bitfield instead of a string based ID

To follow on from #1, internal variable processing could be greatly simplified to use a int/bit style message ID (we aren't going to have more than 255 internal messages for example).

Due to this being a known message for internal use, use the value directly as "the array index", removing the search entirely.

This reduces execution time and code space. Its highly unlikely that memory usage would be negatively impacted either.

Version information in uint32

We agreed on using the semvar style of version for the embedded library, but embed the 3 values into 24-bits of a uint32.

This can be accomplished by shifting and bitwise OR each part of the version together (and simplifies memory allocation/handling etc).

At runtime, this works fine.

When including a C library into the C++ arduino envinronment, there seems to be an issue with the handling of bitwise maths in the define.

Binary Protocol Details

This issue to track some outstanding details with regard to use of the protocol.

Current testing implementation supports arrays, all types and structures nicely.

Function callbacks are probably best done by sending the function pointer back and forth, i.e the microcontroller will tell the UI that "save" is available with pointer 123dead1, and the UI should remember this and just send it back when performing the 'callback'.

This way the micro can check the type and instead of saving it into memory, we call it with the *(void *)&function approach.

Can't easily write out dev eUIobject array to UI for message discovery.

The newer offset handling of larger types and intelligent packet forming at the application layer means some of the previously manually formed internal callbacks could be simplified.

This has been done in dc7d39c for the full variable output, but the 'just message ID' list is not so simple.

While it was trivial to output the developer structure, I realised the msgID field in the structure is actually the pointer to the character array in flash mem.

As a result, uploading raw structure data provides useless msgID and payload pointers, then the mildly useful type and length fields intermingled.

I see the following choices:

1. We don't actually need msgID only advertisements, and use the 'at build time' list from the developer in production
2. We just request the variables with their data, which might not be overly problematic for lower variable count setups.
3. We continue manually forming some customised data type for the UI to get the list of msgIDs.
4. (not really a choice I'd consider) we store the msgID characters in the actual structure, allowing this functionality to work. We'd still be sending some useless pointer information though.

Rework type lists

The idea of shifting the 'not actual types' to the first set of types is appealing.

I.E. Callback, Custom, Offset, etc are first, followed by the standard types

Refactor application layer 'settings' structure data

Currently passing a stripped down settings structure at the application layer to reduce effort for sending simple messages.

I removed the ack bit earlier today with different ack handling, and the type is now done just before send_tracked calls the protocol layer for output.

As such, the only data remaining is the internal/external, and the response bit, which is entirely unused at this stage.

Saleae Logic Analyzer plugin

In the same vein as the wireshark plugin, a Saleae Logic Analyzer plugin would also be neat.

https://github.com/aperepel/saleae-pjon-protocol-analyzer

Investigate CRC hardening methods

Currently the XOR'ed checksum is good for a sanity check on malformed messages, but a few areas could be improved:

• If the CRC process XOR's a byte of equal value to the running CRC value, you get 00. Does this affect CRC credibility given we look balanced 00 at the end of the message parse for validity.
• Is there any merit in incrementing some additional counter or adding a bit for each XOR operation?
• Are there any other fast, easy to understand checksum methods available with similar overheads and better protections?

Work out ACK behaviour

We've added the idea of the UI (or micro) requesting an ACK on successful parsing of a packet. Useful for confirming saves etc.

Unknown protocol definition (for internal use at least) on how to behave when sending an ACK for a given packet.

Options include:

1. Send back a "ack" message with no payload, and the UI assumes it belongs to the last message transmitted.
2. Send back an "ack" message with the messageID as the payload
3. Send back a "originalID" packet, with a custom type/payload for ack.
4. Send back an "originalID" packet, with the microcontroller's most recent understanding of that variable (may not apply to callbacks or unchanging data very well).
5. A few permutations on the above options, with lower levels of efficiency.

Actually write unit tests

This really does need to happen, as much as I keep putting it off...

Keep track of loop times in a variable that I can request

The UI picks transports based on a cost function that's currently only based on latency, it would be good to also be able to use changes in loop time as an indicator of load.

There doesn't need to be any units, it could be measured in nanoseconds or milliseconds, and be of any number type, it just needs to be a linear indicator where a bigger number means more load. I'll use deviation from the average as the actual indication.

Restructure interface management to allow automatic output switching

We want the ability for a UI to inspect/request information about other potential transports to the device, and I am forecasting a need for better link management on the embedded side as usecases expand in scope.

With the concept of an 'interface manager' we can then allow the developer to just 'send a message' and the link/transport should be auto-magically handled, instead of requiring the dev to pass the interface (and therefore monitor links, check if there is something on the other end, etc).

This should allow the UI to then gain hints, modify properties of the interfaces, or redirect traffic to another interface (setting the 'primary' interface).

• Library can track an array of interfaces
• Move output function to interface metadata instead of inside state
• Optional developer callbacks when a message is parsed
• Metadata alongside interface to specify link style etc
• Ability to report available links to the UI during handshake
• Ability for UI or developer to specify a given interface as 'default'
• send_message variant which chooses a default interface

Work out how to escape user space \lf or \cr characters

Carriage return and linefeed characters are used by the serial hardware to 'end' a packet in transit.

If the user sends a string with these characters, we need to ensure they are encoded correctly so the hardware won't internally trigger the callbacks for end of line behaviour.

Interacting with developer variables break querying internal

If I send 01 05 00 08 61 73 15 ed 04 (the as query packet) it replies correctly.

Then I query the led parameter with 01 44 00 0c 6c 65 64 64 1b 04 it replies correctly.

Then I send 01 05 00 08 61 73 15 ed 04 (the as query packet) again, I receive an error number 3.

---

If I restart the micro, query led, then ask for as, I get the same error for the as query.

---

If I restart the micro, set led with no query or ack, it breaks on as once again

---

I've tried sending the packets from outside the UI and I can replicate the behavior.

'dml' packets should have a type of custom

dml packets get caught by the uint8 transform when they should be considered by a custom one.

They should have a header like this:

  euiHeader_t dmlHeader = {.internal = MSG_INTERNAL,
                           .customType = MSG_TYPE_CUSTOM,
                           .reqACK = MSG_ACK_NOTREQ,
                           .reserved = MSG_RES_L,
                           .type = TYPE_CUSTOM_MARKER};

Requesting an ack replies with a payload

When requesting an ack for a variable, the micro replies with the payload as well.

It happens with both internal and developer variables.

It correctly replies with the ack number.

I'm pretty sure this bug is known and just due to this stuff not being quite finished / hooked up:

https://github.com/Scottapotamas/electricui-embedded/blob/master/electricui.c#L117

Decide on error handling behavior in parser

When the microcontroller parses the message, checks for expected msgID length, payload length, and spurious characters where we aren't expecting them.

These edge cases can generate errors if needed.

What is the official expected response from a party when a CRC fails, or the messageID recieved doesn't match any tracked ones?

Tracked object search checks the entire array regardless of early successes

There is room for a performance improvement here, once a pointer has been found, don't keep checking the array.

https://github.com/Scottapotamas/electricui-embedded/blob/766bebb7a6e1d381d8660d2c049b741ab7aaeba6/electricui.c#L43

Generate offset metadata packet prior to offset burst

Discussion from the other day (slack and in-person) is suggesting that the micro should advertise the start and end offset addresses prior to sending an offset packet, primarily for cleaner memory allocation, but also to allow for progress notifications.

Work out alternative/solution to designated initalisers for C++ support

I was planning to use designated initialises (a C idea) to handle the static compile time definitions of the messages, their ID/props/payload pointers.

This would allow for the following style of definition:

const euiMessage_t msg_store[] = {
    {.msgID = "wsr", .type = TYPE_UINT8, .payload = &example_uint8, .internalmsg = MSG_INTERNAL },
    {.msgID = "wsg", .type = TYPE_UINT16, .payload = &example_uint16 },
};

using lazy designation to only require input of relevant fields such as header information. It also allows for a defined enum array to then define human readable names to each object for compile time syntactic sugar like:

[ _RED_LED ] = {.msgID = "wsg", .type = TYPE_UINT16, .payload = &example_uint16 },

where _RED_LED is used as an argument in a function call to update the UI for example.

Other benefits include: unlisted members being set to a default, doesn't care about order in the structure definition.

---

Turns out C++ doesn't support designated initalisers...

So I'm not sure what I can do to support both C and C++ with the same data structures etc.

Offset packets for partial range transmission

The current embedded implementation receives any arbitrary offset and data, which allows the UI to send just a small chunk of a far larger data structure. The outbound offset support currently only supports sending the entire large structure.

Consider the following:

• Arbitrary input and output range addresses.
• Decouple the offset message generation from the general response code in the application layer

Include example for big endian micros?

We assume that everything is in little-endian due to most modern processors being LE.

On the off chance someone is BE, we might just want to make a note of "set the BE/LE flag in the UI" and "watch what happens when the protocol breaks" stuff...

Rework CRC to handle 'running' checksums instead of buffer processing

We don't store a buffer for the inbound message, so we need to perform per-byte CRC calculations as data is fed in from the buffer.

This is probably a good idea for the outbound generation as well, as we technically don't need an outbound buffer there either...

If I request an ack on an offset packet, reply with the ack with the same offset

I need to know which offset has been ack'd so I can maintain which slices have been successfully received.

Introduce feature flags and developer configurable settings

We want to allow developers to have more control over the level of functionality, and the ability to tune the embedded library to suit their particular situation.

The general range of hardware will vary from an ATTiny (8k flash, 512B ram), typical 'arduino' cores like the 328p (32k flash, 2k ram), all the way to the higher end ARM cores (256k-2mb flash, >192k ram). Additionally, clock/mips will vary dramatically between cores.

Therefore at a minimum, a few features should be optional to reduce code-space requirements:

• Offset packets
• Error handling/reporting
• UI facing internal variables

RAM usage is affected by the various buffers for data, along with state information:

• inbound payload buffer should be a configurable size (or passed in by the developer?).
• inbound msgID buffer should be configurable size
• [ ]

This will also pave the way for more advanced functionality like message queues, micro side retry, function callbacks per variable.

Use different preamble character to distinguish user or internal messages

In the 0.1 MVP a '&' was used to signify the start of a message.

The message is then searched for in the variable array to find the corresponding variable/callback etc.

This is fine, but we can save a reasonable search by immediately narrowing the search-space to user or internal messages.

Speed/QoL improvements to eui object declarations

Currently, the developer has to generate an array of eUIObjects which provides us the metadata to use those variables (pointer, type information), which looks like:

{ .msgID = "cback", .type = TYPE_CALLBACK,.size = CALLBACK_SIZE,        .payload = &example_callback },
{ .msgID = "ch",    .type = TYPE_CHAR,    .size = sizeof(example_char), .payload = &example_char     },
{ .msgID = "si8",   .type = TYPE_INT8,    .size = sizeof(example_int8), .payload = &example_int8     },

The key problem that needs solving is reduction in boilerplate/speed improvements, namely the visual width and amount of room to make mistakes.

This is why pre-processor macros are a thing...

We have a set of rigidly defined types which should be OK with hardcoded sizes, and can allow the developer indicate element size manually for custom types. An example of usage:

EUI_UINT8( "led", &led_brightness ),

I don't believe a generic single macro which identifies type is practical/possible...

This has been loosely implemented at a library level in b4d40b0 and it builds without error/tests pass.

Before merging into master:

• Validate on hardware.
• Update example arduino projects.
• Consider header name.
• Potentially shift other declarations/macros to it.
• (Attempt to) write unit tests which validate the macro isn't broken.

Determine how user defined structure can be passed to the UI

User structs are going to be a pain, but probably quite convenient for pre-existing 'objects'.

Essentially, its still just a bunch of bytes. So...

• Write a function that can take a structure pointer and spits out a compliant 'payload'.
• Work with UI overlord to determine how they could define a custom datastructure based on this object (ideally using the same C syntax?)/
• See if it works
• Hope they don't do stuff like pass pointers and expect them to resolve...

Ack Behaviour

This is mostly due to a general unease I feel about ack packets not being explicitly defined as such, a null payload seems too implicit.

Current Way

aH pF -> write with ack request
aL pE -> ack response

aH pE -> callback, ack request
aL pE -> callback no ack request

There's a scenario where two devices with the same messageIDs, and callback messageIDs, call a callback on one, request an ack, and then it calls the callback on the other.

I like the idea of two devices being able to communicate without having to check for this edge case.

Scott's slack chat suggestion (with minor change to allow current heartbeat functionality)

qL, aL, payload > 0, -> write
qL, aL, payload = 0, -> type = ack, response
qL, aL, payload = 0, -> type = existing - do nothing?
qH, aL, payload >= 0, -> write payload contents, respond with variable's payload.
qL, aH, payload > 0, -> write and then output ack response.
qH, aH, payload > 0, -> should this ever happen? (M: it's weird but I guess permitted)

So pseudocode for this would be:

if !q && type == callback: 
  call callback
elif payload len > 0:
  write contents

if qH:
  reply with query packet

if aH:
  reply with ack packet

if type == TYPE_ACK
  received an ack packet

This would allow devices to communicate to each other as above.

I have a general "shouldn't the types be regarding just the payload" idea about this.

If the ack number information was stored in the payload this would make more sense to me? We could lengthen the messageID length / payload length / more types / reserved bits?

Other suggestion

aL, qL -> blind set (if callback, call it)
aH, qH -> request ack for this (if callback, call it)
aH, qL -> this is the ack response (if callback don’t call)
aL, qH -> request a query response for this packet (if callback, reply with information but don’t call)

if q == a && type == callback: 
  call callback
elif payload len > 0:
  write contents

if qH: 
  if aH:
    respond with ack packet
  else: # aL
    respond with query packet
else:
  if aH:
    received an ack packet

We avoid the situation where someone both queries and ‘requests an ack’ for a packet at the same time.

We don't use up a type.

I'm having trouble backing up this idea with any more arguments other than "this feels right", which I don't think is very actionable?

Embedded library has a 254 tracked message limit

Need to consider if this is a reasonable limit or not.

https://github.com/Scottapotamas/electricui-embedded/blob/766bebb7a6e1d381d8660d2c049b741ab7aaeba6/electricui.h#L43

Deal with payloads that are longer than the inbound buffer

There are a few things that can be added to the serial transport to handle this potential error.

• When a inbound payload length exceeds that of the data, pass an error up the tree.
• Report the error to the UI as a "longer than inbound payload".

C++ compiler forbids sizeof() on function_ptr

Apparently ISO C++ prevents sizeof use on a function.

This is problematic due to the use of this functionality inside our extern escaped C code for internal callbacks.

Internal Callback that sends all the developer payloads

The UI will request 'dm' for the list of messages, but the UI also needs the payloads of those messageIDs.

We need a callback that the UI can request for the micro to send the contents of all developer variables.

(These are split up so I can have a loading bar)

Assess viability of larger checksum space due to increased efficiency of transport

The 0.1 MVP library used ascii strings to represent the 8-bit checksum. This meant that up to 3 bytes were sent when one would have sufficed (and hex would have been an improvement to 2 bytes for the same space. 20/20 hindsight).

Using binary for the new protocol, we will mandate a fixed and constant number of bytes/bits. We could use one byte for the checksum to reach feature parity, or increase this to n-byte if we feel like worrying about collisions.

Tasks for this issue:

• Assess if 8-bit is enough to ensure consistent results with the protocol with wireless/high speed datarates
• Measure impact of adding additional bytes.
• Implement user defined number of bytes for the checksum on this end (and then presumably the UI as well).