The code compiles again, and it seems to work as it did before. There's
still a lot to do (we still have a mess of globals), but the PE
correctly calls the new DPM functions via the list of callbacks in the
configuration object.
For now, the new <pdb.h> file only has a function to start the library,
based on the old start_pd function from main.c. It'll eventually be all
a user needs to include, but there's a lot more refactoring to do.
/!\ BROKEN BUILD /!\
This commit moves the library code to the library directory. This
causes the firmware to not compile. A few changes were made to reduce
the nubmer of errors, but some errors just aren't going to go away until
I go ahead with the planned changes.
Only set the output in trans.sink when not min pwr
When we receive a GotoMin message, we immediately go to lower power
before entering the PE_SRC_Transition_Sink state. Therefore, the call
to pdb_dpm_output_set was redundant in this case. Now the call isn't
made in this case, removing the redundancy. This also leads somewhat
into the design of the new library.
The "storage" source code files contained lots of stuff with "config" in
the name, but nothing named "storage". This was unlike every other part
of the firmware, so I renamed the files to "config".
Now the call to fusb_get_typec_current is made in the policy engine,
then passed to pdb_dpm_evaluate_typec_current. This is more consistent
with pdb_dpm_evaluate_capability, improving the PD Buddy DPM API.
That's right, I'm thinking about API design now for when I split the
code into a library and application.
Now that we cache the configuration location in the configuration code
itself, we don't need to cache it elsewhere. This makes the DPM code
for evaluating Type-C Current much cleaner.
Until now, the valid configuration object's location was not stored, so
it had to be looked up every time pdb_config_flash_read was called. Now
the valid object's location is stored in config_cur, which is updated
every time the configuration's location changes. This means that
pdb_config_flash_read only has to do the full routine the first time
it's called, or after pdb_config_flash_erase is run. Now other parts of
the configuration won't have to cache the location themselves anymore.
The cfg_set variable was not necessary after all, since testing if
cfg == NULL works just as well as testing if !cfg_set. Actually a bit
better, since it prevents a null dereference we did by mistake before.
I should find out if the null derefernce hurts anything, because if it
does I should make a patch to the 1.1.x series to fix it.
The received Type-C Current advertisement is now kept in a global
variable which can be read by the command shell. When there are no PD
capabilities but there is Type-C Current, the typec_virtual PDO is
reported as the one and only PDO.
This commit adds support for the firmware version extension, as
described at http://www.fwupd.org/developers S. USB Firmware Version
Extensions. This gives a new way of reading the firmware version, which
is apparently preferred by fwupd. I'd kinda like to be able to use
fwupd to update the device's firmware starting with the new minor
version, so this is a first step towards that.
Since the spec says we Shall request no more power than we actually
need, we were violating the spec whenever the output was disabled in
Setup mode. This commit fixes that, requesting low current at 5 V when
the output is disabled.
We do play a little fast and loose with the MAX_CURRENT field, but
that's hard to avoid in a device like this. Well, we could always set
it to 5 A in Setup mode to be safe, but that's not necessarily right
either. At least it's still honest in Sink mode, which I consider more
important anyway.
We don't need to update the source's capabilities when setting our
output state. Now we just send a new Request. For now, that request is
redundant, but soon I want to make the Sink request less current when
the output is disabled. Whether the voltage should be the same or
always vSafe5V in that case, I haven't decided yet. There are
advantages to each, but the difference is so minor in terms of using the
device that I don't see a reason to let the user choose, which of course
puts the decision on me.
It was never really necessary to update the source capabilities before
making sending a Request message. It was convenient though, since we
didn't store the previous Source_Capabilities message. Now we do, so
it's possible to make a new Request based on that.
This commit makes the Sink do just that. It doesn't follow the spec's
state diagram for this, but it's a minor change that works better with
this firmware's design, so I don't mind. There's a comment explaining
that too, so future folks won't be confused by it either.
The `PDO n:` and `type: blah` lines have now been combined into a single
`PDO n: blah` line. This saves one line per PDO, and still makes plenty
sense.
When we were getting power from Type-C Current advertisements, we used
to not update the location of the configuration object. This was done
to save time between polling the Type-C Current, and was not a problem
until we started trying to do Power Delivery in Setup mode. As the
comment correctly said before, when we were trying to get power, the
location of the configuration object wouldn't change, so there was no
need to find its location more than once.
Now that the location of the configuration can change while we're
getting power, we need to check if that's happened. This can be easily
done by checking the configuration object's status. If the status is
not VALID, we get the location of the new configuration object.
It prints the most recently advertised PDOs in a reasonable format. The
types it doesn't know about are printed as hex so that someone
knowledgeable can still get the information they need. For now, it only
knows about fixed PDOs, so variable and battery PDOs will be shown as
hex.
Since this means we remember the most recent Source_Capabilities, it's
now becoming possible to send a new Request without sending a
Get_Source_Cap message first. That's not actually done yet, but I'd
like to do it that way. I'd also at some point like to make a
lower-current Request when the PD Buddy Sink's output is disabled, but
again, that's still to come.
Just as specified last night. This represents the completion of a big
part of the new interactive Power Delivery features of the shell.
Next up: printing the source's advertised PDOs.
Update requested power after writing configuration
The PD Buddy Sink now requests new power after configuration is written.
This means that in Setup mode, the voltage and current can be
re-negotiated on the fly. Cool, huh?
It's still impossible to turn the output on and off from the shell. New
commands will allow that soon enough, though. Also, I'm seeing some
weird behavior when switching to/from 5 V (power shuts off entirely
sometimes), but I suspect that's a quirk of the source I'm using (Asus
USB 3.1 UPD Panel) and not the PD Buddy Sink itself. I plan to make or
buy a USB power/data splitter to verify this.
The DPM used to always set the LED to indicate the PD status. This
caused fighting when the PD threads were run simultaneously with the
shell, making the LED show different things depending on what commands
the user ran. Not cool!
This commit adds a bool pdb_dpm_led_pd_status, which prevents the DPM
from setting the LED when set to false. This commit also sets it to
false before starting the PD threads in Setup mode, allowing the shell
to be in full control of the LED. Right on!
A good first step towards the upcoming 1.1 release, this commit runs the
Power Delivery threads in Setup mode. This required a slight change to
the shell to make it non-blocking, as otherwise the PD threads would
never get to run.
There's still a lot to do! The shell and the PD threads fight over
control of the LED in Setup mode. There's no way to make the PD threads
re-negotiate the required power. There's also no way to turn the output
on or off from the shell. None of these changes should be too major,
but together they'll be pretty cool when they're all done.
The shell main loop function and text input function were dreadfully
devoid of comments. This simply could not continue, so I added a
sprinkling of comments to each, as well as to important sections of the
file. Further, I changed the special character values to look like
characters rather than numbers, which should aid in understanding.
Now the error behavior for set_v and set_i is well-defined by the
documentation: they print nothing on success and an error message on
failure. The range of valid values is now [0, 20000] for set_v and
[0, 5000] for set_i, matching the voltages and currents that can be
provided by USB Power Delivery.
Bumped version number to 1.0.1, reflecting this bugfix.
The iSerial descriptor now holds the firmware version number. This is
1.0.0 since I don't expect any major API changes any time soon. The USB
descriptors have been documented in docs/console_config.md so there
will be no questions as to how to identify the devices and what their
"serial numbers" mean.
The VID:PID pair 1209:9DB5 is now officially assigned for the PD Buddy
Sink. Thanks, Arachnid! Now the firmware uses its proper PID instead
of the testing one.
As the name suggests, it clears all the flags in the configuration
buffer. This provides a way to easily set all the flags to a known
state, which is nice for implementing libraries that configure PD Buddy
Sink devices.
Now we correctly set the GiveBack flag and minimum current fields in the
Request message when GiveBack is enabled. Also when GiveBack is
enabled, we correctly handle GotoMin messages by lowering our power
consumption, completing negotiation, and Requesting our normal power
periodically until the power supply Accepts our Request.
At least, I believe it's correct. I don't have any hardware that sends
GotoMin messages, so I can't test it other than that it doesn't break
anything with the GiveBack flag set to 1 or 0 when the source doesn't
ask to reduce power.
As the name says, it toggles the GiveBack flag in the configuration
buffer. The rest of the firmware doesn't use the flag for anything yet,
but it is correctly saved in flash.
We used to free the Request we made after transmitting it. However, for
GiveBack to work properly, we need to be able to make that request
again. To do it without having to request the Source_Capabilities
again, it makes sense to keep the Request message in memory. We have
more messages in the pool than we need anyway.
The policy engine is supposed to transition to the
PE_SNK_Transition_to_default state from any state when hard reset
signaling is received. While it still doesn't do that exactly, in every
state that already waited for events, the PDB_EVT_PE_RESET event is also
checked, with the appropriate transition being made if the event is
received.
My justification for not checking for the event in other states is that
if there's no wait, the reset will be received soon by another state
anyway. Besides, if there's no event handling already, where exactly
would be best to insert a test for the event?
I think so, anyway. Testing with a heat gun didn't result in the Sink
resetting itself, so I'm not confident that I understand the FUSB302B's
over-temperature sensing mechanism.
I must point out though that I'm not confident that this
over-temperature protection is even useful. The PD Buddy Sink doesn't
get hot on its own, and if it did, it might not even be from the device
it's powering, making a hard reset useless. The spec says that we Shall
implement over temperature protection though, and that we Shall send a
Hard Reset message when it engages, so even though I take issue with the
requirement, I'll try to do what the standard says I Shall.
The README now outlines the features of the PD Buddy Firmware.
A bit of documentation was added to main.c, along with renaming the
pd_buddy() function to sink().
When there is a positive or negative transition of Vbus, we Shall reduce
our power consumption to no more than 2.5 W beforehand. This can be
easily achieved in all cases by simply turning off the output. This was
implemented, so that's one fewer Shall we aren't doing.
In the process, I made some API changes for the Device Policy Manager.
No more output_on() and output_off(), now it's output_set(true) and
output_set(false), plus a special case output_default() used during a
hard reset. This much better matches how these functions are used by
the policy engine.
