digistump / OakCore

Removed compiler warnings and minor code comment changes

… conversion from string constant to 'char*'" warnings I was getting from the Arduino IDE, probably due to recent changes to the compiler toolchain, and showing warnings it never used to.

Ammended some of the comments to better reflect the purpose of the code - just for the helper function calls in the setup block.

Added LED_BUILTIN to all three variant types

…es that only specify esp8266. The encourages the Arduino IDE to use these libraries in preference to any conflicting libraries supplied with the Arduino IDE, and also removes any warnings about 'library supports esp8266 architecture but compiling for oak'.

…e the built-in LED via this constant, not BUILTIN_LED.

Restore blink during OTA update

The previous change to event_loop() to process all events accumulated
in the buffer, rather than one per call, had the inadvertently prevented
the OTA update blink. During the OTA update, data comes in fast enough
that the buffer never empties, so the first call to event_loop() doesn't
finish until the update is done. As a result, the blink loop at the end
of handle_update_begin() only runs for one iteration.

This commit adds a 20 ms time limit after which event_loop() will stop
processing further events and return.

Various fixes for communications with the Particle Cloud

The pClient.available() check prevented the event_loop(message) call
unless there was a received message to be processed. I overlooked the
fact that even if there are no incoming messages, it will send a ping
every 15 s.

Currently, event_loop() processes only one message per iteration, even
if there are more in the receive buffer. This means that messages can
build up in the receive buffer (I have observed this happening during
the Particle Cloud handshake), which might be problematic.

It's not clear if this this is intentional or an oversight. The
overridden version of event_loop() which waits for a specific message
does loop. I don't see any negative consequences of processing
everything in the buffer, so this commit modifies event_loop() to do so.
It yields between messages and I have also added a corresponding yield
to the specific message event_loop().

Otherwise, the connection will remain open, and the enclosing if block

if(!pClient.connected()) {
...
}

will not be called again. This means the firmware does not try to
connect again.

It's a good idea to randomize the port used, otherwise a rebooted Oak
will try and reconnect on the same port, which may result in a long
delay if the connection was not closed before the reboot, as mentioned
by @jldeon here:

#54 (comment)

I believe I have observed this issue in practice on a few occasions,
where it would take exactly five seconds for the first data to come
through the connection, which seems too constant to be a random delay.
As the read timeout in blocking_receive() is 2 s, this is obviously
problematic.

As most users will be behind routers running NAT, it will be the router
settings, rather than the Particle server settings, that affect how
reconnections from the same source port are treated.

With the existing code, the tcp transmit buffer can fill up during the
Particle Cloud handshake because there is no yield to let the ESP8266
send out data. This results in the tcp connection being dropped and
a new one opened, with the handshake repeated again. This manifests as
the Particle Cloud showing the device cycling online-offline every
second or so.

The buffer must be right on the threshold of filling up, because this
only happens for some people, and only some of the time. I suspect it's
sensitive to the local Wifi environment (latency, interference, packets
from other devices, etc).

Yielding solves this issue. There are multiple points in the code where
the yield(s) could be inserted. I chose to put them after the send in
blocking_send() and before the receive in blocking_receive(), since by
definition these functions should block until the data is sent or
received, and there is an inherent expectation that these functions
can take some time to execute.

Allow the current mode to be queried

Listens for an an 'oak/device/reset event' with data 'mode?' and then
responds with an 'oak/device/mode/<current_mode>' event, where
<current_mode> is 'config', 'user' or 'update'.

Enable cloud serial comms to individual devices

The Particle Cloud API and ParticleJS do not appear to allow publishing
events to specific devices. This change works around that limitation by
having the Oaks listen to /oak/device/stdin/:deviceid and
/oak/device/reset/:deviceid so that they can be individually addressed.

The Oaks are also still listening to /oak/device/stdin and
/oak/device/reset so that all of one's Oaks can be addressed
simultaneously if desired.

Cloud serial fixes

It looks like lastCloudEvent was intended to be used for rate limiting,
since it's not used anywhere else, but it was being updated every time
spark_process() ran, not just when data was actually sent. Changed it
to update only when data is sent.

The send buffer was not being null terminated, so spark_send_event would
keep reading until it hit a zero byte, resulting in unexpected
characters being sent at the end of the message. Added this null
termination.

Also, the previous head and tail indexing approach meant one byte in the
buffer would never be used because an empty buffer and a full buffer
both have head == tail, so it would be impossible to tell the
difference. Changed to head and count indexing which avoids this issue.
Since the maximum event data length is 255 bytes (see Messages::event()
in messages.cpp), reduced MAX_SERIAL_BUFF by one to match, as we can now
use all bytes of the buffer.

The cloud serial read, write and peek functions did not check whether
begin() had been called first. If not, the read/write/peek would lock up
the ESP8266 with a fatal exception.