trade: Fixes, features, and improvements

This commit wraps up a number of changes that aim to improve the trade
process, its robustness, add some features, and fix some bugs.

Change list in rough order of importance:

Better pattern matching of incoming data for each trade step, provides
more robust logic during trading and more correctly sync's states
between the Flipper and gameboy.
Fixes #20 as a side effect.

Implement trade patch list. This allows for the Flipper to both send and
receive patch data. The patch list is used by Pokemon to patch what is
considered to be a NO_DATA_BYTE (e.g. data from the follower was not yet
ready) with another byte, and then track that byte's index. The other
side then restores that byte to what it should be.
Fixes #18

Ability for the Flipper to correctly receive the Pokemon traded to it
and put that Pokemon in the Flipper's trade buffer. This allows for
trading back and forth (e.g. in the case of a Pokemon that evolves with
trade, trading to the Flipper and back will trigger an evolution on the
Gameboy), as well as the following feature.

Ability for the Flipper to back out of the trade menu, modify the
current Pokemon, and re-enter the trade menu without having to
completely reset the Gameboy and re-enter the Cable Club.
Fixes #19

Completely isolates Trade's context and scope. The _alloc() function now
returns an anonymous pointer that means nothing to the rest of the
application. However, this does require a bit of juggling as the main
application is responsible for setting up the View at the start.

Adds a huge comment block in the trade source outlining the current
understanding of the actual trade data protocol. Also adds specific
comments for more context specific details through various trade states.

Changes to how the draw callback is called. In order to correctly update
the canvas during the actual trading with a graphic and LED changed
every 250 ms, the timer to call the update was changed to only run every
250 ms. However, Flipper OS does not guarantee that this is the only
draw update call, and may issue a draw update at any time. The view
model now tracks what the LED state should be, and our timed update
routine callback is the only place this LED state is toggled. This
forces the trade animation to always be sync'ed correctly no matter how
often the Flipper OS calls it.

Clean up state tracking overall. There are now two states that are
tracked, the Gameboy/link state, and the actual trade states. The
Gameboy state still has a bit of overlap with the trade states, however,
it combines what was the link state in to this. This also allows
elimination of additional bools that were used for state tracking in
parallel. State transitions and meanings should now be a bit more
straightforward.

CLK pin now implements an interrupt on either edge. The ISR was updated
to shift data out on the falling edge, and read data in on the rising
edge. This eliminates any need for delays while in an interrupt context
as well as mimics Gameboy behavior by matching the setup/hold times.

Remove use of magic numbers as much as possible. Bytes to/from the
Pokemon game now use macros for most of the bytes. Many of these were
pulled from https://github.com/pret/pokered defines.

Clean up cycle counter to real-time maths. Copied general paradigms from
Flipper onewire code. This also includes the bit counter timeout and now
ensures correct timeouts are measured between transmitted bytes.

Author: kbembedded

pokemon_app.c

@@ -1863,17 +1863,23 @@ const NamedList type_list[] = {
     {},
 };
 
+int pokemon_table_get_num_from_index(const PokemonTable* table, uint8_t index) {
+    int i;
+
+    for(i = 0;; i++) {
+        if(table[i].index == index) return i;
+        if(table[i].name == NULL) break;
+    }
+
+    return 0;
+}
+
 int pokemon_named_list_get_num_elements(const NamedList* list) {
     int i;
 
     for(i = 0;; i++) {
         if(list[i].name == NULL) return i;
     }
-
-    /* XXX: Would be faster to do something like this, but, can't easily do
-     * that using the current pointers. Might be able to clean this up later?
-     */
-    //return sizeof(type_list)/sizeof(type_list[0]);
 }
 
 int pokemon_named_list_get_list_pos_from_index(const NamedList* list, uint8_t index) {
@@ -1884,7 +1890,7 @@ int pokemon_named_list_get_list_pos_from_index(const NamedList* list, uint8_t in
         if(index == list[i].index) return i;
     }
 
-    /* XXX: This will return the first entry in case index is not matched.
+    /* This will return the first entry in case index is not matched.
      * Could be surprising at runtime.
      */
     return 0;
@@ -1898,7 +1904,7 @@ const char* pokemon_named_list_get_name_from_index(const NamedList* list, uint8_
         if(index == list[i].index) return list[i].name;
     }
 
-    /* XXX: This will return the first entry in the case index is not matched,
+    /* This will return the first entry in the case index is not matched,
      * this could be confusing/problematic at runtime.
      */
     return list[0].name;
@@ -1912,9 +1918,6 @@ void pokemon_trade_block_set_default_name(char* dest, PokemonFap* pokemon_fap, s
     /* Walk through the default name, toupper() each character, encode it, and
      * then write that to the same position in the trade_block.
      */
-    /* XXX: The limit of this is hard-coded to a length of 11 at most. This may
-     * be a problem down the road!
-     */
     for(i = 0; i < 11; i++) {
         pokemon_fap->trade_block->nickname[0].str[i] = pokemon_char_to_encoded(
             toupper(pokemon_fap->pokemon_table[pokemon_fap->curr_pokemon].name[i]));
@@ -2117,9 +2120,11 @@ static TradeBlock* trade_block_alloc(void) {
     /* OT trainer ID# */
     trade->party[0].ot_id = __builtin_bswap16(42069);
 
-    /* XXX: move pp isn't explicitly set up, should be fine */
-    /* XXX: catch/held isn't explicitly set up, should be okay for only Gen I support now */
-    /* XXX: Status condition isn't explicity let up, would you ever want to? */
+    /* Notes:
+     * Move pp isn't explicitly set up, should be fine
+     * Catch/held isn't explicitly set up, should be okay for only Gen I support now
+     * Status condition isn't explicity let up, would you ever want to?
+     */
 
     /* Set up initial level */
     trade->party[0].level = 2;
@@ -2180,8 +2185,12 @@ PokemonFap* pokemon_alloc() {
         pokemon_fap->view_dispatcher, AppViewSelectPokemon, pokemon_fap->select_view);
 
     // Trade View
-    pokemon_fap->trade_view = trade_alloc(pokemon_fap);
-    view_dispatcher_add_view(pokemon_fap->view_dispatcher, AppViewTrade, pokemon_fap->trade_view);
+    /* Allocates its own view and adds it to the main view_dispatcher */
+    pokemon_fap->trade = trade_alloc(
+        pokemon_fap->trade_block,
+        pokemon_fap->pokemon_table,
+        pokemon_fap->view_dispatcher,
+        AppViewTrade);
 
     return pokemon_fap;
 }
@@ -2193,8 +2202,8 @@ void free_app(PokemonFap* pokemon_fap) {
     view_dispatcher_remove_view(pokemon_fap->view_dispatcher, AppViewSelectPokemon);
     select_pokemon_free(pokemon_fap);
 
-    view_dispatcher_remove_view(pokemon_fap->view_dispatcher, AppViewTrade);
-    trade_free(pokemon_fap);
+    /* Also removes itself from the view_dispatcher */
+    trade_free(pokemon_fap->view_dispatcher, AppViewTrade, pokemon_fap->trade);
 
     view_dispatcher_remove_view(pokemon_fap->view_dispatcher, AppViewMainMenu);
 

pokemon_app.h

@@ -44,22 +44,12 @@ struct named_list {
 
 typedef struct named_list NamedList;
 
-typedef enum {
-    GAMEBOY_INITIAL,
-    GAMEBOY_READY,
-    GAMEBOY_WAITING,
-    GAMEBOY_TRADE_READY,
-    GAMEBOY_SEND,
-    GAMEBOY_PENDING,
-    GAMEBOY_TRADING
-} render_gameboy_state_t;
-
 struct pokemon_fap {
     ViewDispatcher* view_dispatcher;
 
     /* View ports for each of the application's steps */
     View* select_view;
-    View* trade_view;
+    void* trade;
 
     /* Scene manager */
     SceneManager* scene_manager;
@@ -105,6 +95,8 @@ typedef enum {
     AppViewExitConfirm,
 } AppView;
 
+int pokemon_table_get_num_from_index(const PokemonTable* table, uint8_t index);
+
 int pokemon_named_list_get_num_elements(const NamedList* list);
 
 int pokemon_named_list_get_list_pos_from_index(const NamedList* list, uint8_t index);

pokemon_char_encode.c

@@ -3,7 +3,6 @@
 
 #include "pokemon_char_encode.h"
 
-/* XXX Current text_input module only offers alnum and space input */
 char pokemon_char_to_encoded(int byte) {
     switch(byte) {
     case 'A':

pokemon_data.h

@@ -13,6 +13,7 @@
  * GB/Z80. e.g. a uint16_t value of 0x2c01 translates to 0x012c.
  * Need to use __builtin_bswap16(val) to switch between Flipper and Pokemon.
  */
+/* This is 44 bytes in memory */
 struct __attribute__((__packed__)) pokemon_structure {
     uint8_t index;
     uint16_t hp; // Calculated from level
@@ -50,7 +51,9 @@ struct __attribute__((__packed__)) name {
     unsigned char str[11];
 };
 
+/* This is 418 bytes in memory/transmitted */
 struct __attribute__((__packed__)) trade_data_block {
+    /* TODO: Change this to use struct name above */
     unsigned char trainer_name[11];
     uint8_t party_cnt;
     /* Only the first pokemon is ever used even though there are 7 bytes here.

scenes/pokemon_level.c

@@ -19,7 +19,7 @@ static bool select_level_input_validator(const char* text, FuriString* error, vo
         rc = false;
     } else {
         pokemon_fap->trade_block->party[0].level = level_val;
-        /* XXX: Need to recalculate other stats with level updated! */
+        pokemon_fap->trade_block->party[0].level_again = level_val;
     }
 
     return rc;
@@ -28,6 +28,7 @@ static bool select_level_input_validator(const char* text, FuriString* error, vo
 static void select_level_input_callback(void* context) {
     PokemonFap* pokemon_fap = (PokemonFap*)context;
 
+    /* Recalculate all stats from updated level */
     pokemon_trade_block_recalculate_stats_from_level(pokemon_fap);
     scene_manager_previous_scene(pokemon_fap->scene_manager);
 }

scenes/pokemon_menu.c

@@ -38,6 +38,14 @@ void main_menu_scene_on_enter(void* context) {
      */
     scene_manager_set_scene_state(pokemon_fap->scene_manager, SelectMoveScene, 0);
 
+    /* HACK: Since we may have come from trade view, we cannot assume that
+     * pokemon_fap->curr_pokemon is correct.
+     * The proper way to do this would be to instead of tracking curr_pokemon
+     * separately, have it always be derived fro the current trade_block.
+     */
+    pokemon_fap->curr_pokemon = pokemon_table_get_num_from_index(
+        pokemon_fap->pokemon_table, pokemon_fap->trade_block->party_members[0]);
+
     submenu_reset(pokemon_fap->submenu);
 
     snprintf(