Project Spec, refactor.

Gather the track types into a single file, probably TrackTypeRegistry.h
Then the device id problem is also solved, we can make a dictionary to map between track type and device id.

btw can we increase it to 16 + 6 tracks?

we have to change project layout to incorporate more tracks.
The current change was intended for 'drop in' replacement without having to convert every project.
But i'm open to this idea.

ideally we would create 'multiple' grids that are layered on top of each other. 2nd grid for ext or fx ?

ideally we would create 'multiple' 16x grids that are layered on top of each other. 2nd grid for ext or fx ?

That'll probably solve the problem that we don't have a way to individually chain ext tracks with TI

Any idea on a layout to accomplish this.
Do we lay grids out in series... if so we cant easily extend...

Do we lay grids out in series...

You mean, horizontally?

woops. i'm talking about the project file layout now.

we should probably have separate files for separate grids... let the file system handle block layout

oh. ok I got it.
yeah, multi file.

we going to have the progmem for all these new features :D ?

About chain/write:
We have free knobs and a data flow display. For example, the chain page can display:
STEP  SND -> A4       QUANT
0        GRID -> SEQ      8
_ _ _ _ _ _

to indicate that we're focusing on the A4 grid.

we going to have the progmem for all these new features :D ?

That's why I keep coming back to https://github.com/jmamma/MIDICtrl20_MegaCommand/pull/125 instead of straight burning the fuel for more features :p

we can do some pressure test on file loading and sequencing by:

Load full 4k for each track (and ignore the unused part)
Apply the locks multiple times during sequencer run

From https://forum.arduino.cc/index.php?topic=66241.msg485805#msg485805 :

For file I/O, most SD cards can be read at about 250 KB/sec.  The class of card doesn't matter much for reading on the Arduino.
The fastest possible read from an SD card on an Arduino is about 600 KB/sec.  This is for streaming multiple contiguous blocks using the SD multiple block read command.  SdFat does not use this command for file I/O since it has only a one block buffer.

Okay, having separate files actually helps in this case. Multi streams for multi files.

do we create project directories, and automatically scan for .mcl files.
each file = a grid.

Just realised we have another limitation...
Chain mode caches tracks in BANK1 heap. We can only access this data by first copying in to stack. Stack will limit the maximum track size.
We might have to move cache to bank1 then...

Chain mode caches tracks in BANK1 heap

Can you elaborate on that bit?
So it's like, it copies from SD to stack, then from stack to BANK1?

Chain mode caches tracks in BANK1 heap

Can you elaborate on that bit?
So it's like, it copies from SD to stack, then from stack to BANK1?

Yes.
Here's the caching routine for A4 from GridTask.cpp
330             if (a4_track->load_track_from_grid(n, mcl_actions.chains[n].row,
331                                                0)) {
332               a4_temp_track->load_from_mem(n);
333               if ((a4_track->active != EMPTY_TRACK_TYPE) &&
334                   (memcmp(&(a4_temp_track), &(a4_track), sizeof(A4Track)) !=
335                    0)) {
336                 mcl_actions.send_machine[n] = 0;
337               } else {
338                 mcl_actions.send_machine[n] = 1;
339                 DEBUG_PRINTLN("sounds match");
340               }
341               a4_track->store_in_mem(n);
342             }

Global variables use 46486 bytes (81%) of dynamic memory, leaving 10345 bytes for local variables.

We probably still have room in the stack. My last check (when we were narrowing down the ISR bug) was that we had an additional 4KB available during chaining.

So if we can directly load data to BANK1 the problem is gone?

So if we can directly load data to BANK1 the problem is gone?
Possibly.

bumping locks up to 8 on MD.
Sketch uses 235928 bytes (92%) of program storage space. Maximum is 253952 bytes.

Recap the address space layout:
0200-21ff 8KB internal SRAM stack
2200-ffff data(heap). external SRAM BANK0/BANK1

The SdFat SD; object is placed at a5ef @ BANK0 so there's no way to directly read into BANK1.
But we don't have to read to stack either.
Just do a 4KB buffer in BANK0, and write a special memcpy to copy from BANK0 to BANK1, where a small buffer is placed in STACK to save the memory pin from toggling every time.
What do you think?

Recap the address space layout:
0200-21ff 8KB internal SRAM stack
2200-ffff data(heap). external SRAM BANK0/BANK1

The SdFat SD; object is placed at a5ef @ BANK0 so there's no way to directly read into BANK1.
But we don't have to read to stack either.
Just do a 4KB buffer in BANK0, and write a special memcpy to copy from BANK0 to BANK1, where a small buffer is placed in STACK to save the memory pin from toggling every time.
What do you think?

yes that will work.

Another option is to write a custom wrapper for reading from a file into BANK1.
This is done by placing a 512 byte buffer on the stack directly and read block-by-block.

We still need to be able to access/query the BANk1 object from somewhere. So have 4KB allocated in bank0 is probably the way to go.

I haven't made any further changes today.
Collating my thoughts for my next move.

I'm thinking of ways to compress the lock data
We could use
uint64_t param_lockmask[8]
to represent step positions for each parameter lock.
then have a shared buffer for all 8 params.
An incremental counter would keep track of which buffer value belonged to which param at which step.
For example,
for each step:
for (i < num_of_params) {
if IS_BIT_SET(step,param_lockmask[i]) { n++; }
lock_value = buffer[n]
i++
}
Available locks =  buffer_size - 8 * 8 =  256 - 64 = 192
We lose 25% of lock space, but have the ability to apply locks to 8 params.
--
Playback performance penalty would be negligible. Recording however would require shifting the buffer array to insert new locks.
This scheme could be extended to note recording note_conditional/timing etc...
--
Does this make sense? Brain is powering down...

Interesting. I'll think about this.
A good property to exploit is that the total distance between all "neighbor" locks do not exceed 64 or 128, though there can be a lot more locks.

Recording however would require shifting the shared array to insert new locks.

This can be alleviated by using a record queue and an REPLACE queue, and merging the queues into the active track data during playback.
Locks between RECORD tail and RECORD head are newly-recorded locks will be committed into the active track on next playback.
A lock goes into the INSERT queue if and only if:

The RECORD queue or INSERT queue wants to write to the active track in a slot
The slot is occupied by an old lock

For each step, we want to merge both INSERT and RECORD head (if exists) into the active track. If both RECORD and INSERT wants to write, RECORD takes precedence, and this may result in another old lock being queued into INSERT.
It is guaranteed that any INSERT element can be placed before any active track lock after the current playback head, because they are the elements kicked out earlier in the run (and the active track buffer is sorted, naturally).
More intuitively, it's like we are merging two sorted lists using merge sort, but instead of merging into a new array, we are merging into an existing one.

Recording however would require shifting the shared array to insert new locks.

This can be alleviated by using a RECORD queue and an INSERT queue, and merging the queues into the active track data during playback.
Locks between RECORD tail and RECORD head are newly-recorded locks will be committed into the active track on next playback.
A lock goes into the INSERT queue if and only if:

The RECORD queue or INSERT queue wants to write to the active track in a slot
The slot is occupied by an old lock

For each step, we want to merge both INSERT and RECORD head (if exists) into the active track. If both RECORD and INSERT wants to write, RECORD takes precedence, and this may result in another old lock being queued into INSERT.
It is guaranteed that any INSERT element can be placed before any active track lock after the current playback head, because they are the elements kicked out earlier in the run (and the active track buffer is sorted, naturally).
More intuitively, it's like we are merging two sorted lists using merge sort, but instead of merging into a new array, we are merging into an existing one.

So we have a "live buffer" that is being simultaneously populated by incoming locks from the active sequencer track, and locks from record. At the end of the the pattern the live buffer is copied in to the sequencer.
Makes sense!
Ideally we'd collate these ideas in to a generic library that can be used for different data types such as locks, notes, utiming etc...
We need to decide if the added complexity is worth it, and the potential progmem overhead ?
Thoughts?

Another problem that needs solving:
I'd like to generalise load_track_from_grid, store_track_in_grid for all derivative classes.
The issue is the len = (sizeof(GridTrack)); parameter.
  5 bool GridTrack::load_track_from_grid(int32_t column, int32_t row) {
  6
  7   bool ret;
  8   int b = 0;
  9
 10 //  DEBUG_PRINT_FN();
 11   int32_t offset = grid.get_slot_offset(column, row);
 12
 13   int32_t len;
 14
 15   ret = proj.file.seekSet(offset);
 16   if (!ret) {
 17     DEBUG_PRINTLN("Seek failed");
 18     return false;
 19   }
 20
 21   len = (sizeof(GridTrack));
 22
 23   // len = (sizeof(GridTrack)  - (LOCK_AMOUNT * 3));
 24   DEBUG_PRINTLN("reading grid track");
 25   ret = mcl_sd.read_data((uint8_t *)this, len, &proj.file);
 26
 27   if (!ret) {
 28     DEBUG_PRINTLN("read failed");
 29     return false;
 30   }
 31   return true;
 32 }

Is there a simple way we can efficiently recycle the code?
...
I'm thinking, read first 512bytes (assume this is the smallest track size) to obtain the track type (active variable). then hardcode length based on various tracktype and finish the complete  read.

Just want to clarify the purpose of the PR, the various proposed changes and their justifications

We want to be able to change track and project memory layouts to enhance sequencer capabilities and support various track types.
We need to implement track/project versioning and conversion so that projects data can be between maintained between versions.
Some changes to the track layout involve changing how tracks are loaded.
We would also like to be able to load any track type from any slot position. This will allow us to support other track types going forward.
To simplify loading, consolidate all track loading in the parent GridTrack object.

I'll add to this list as details become clearer. I'm working through the code and seeing what needs to be changed. There is a high level of dependency between Grid, Sequencer and Chain mode. So changes to memory layouts will have knock on effects to each of these subsystems.

we going to have the progmem for all these new features :D ?

That's why I keep coming back to #125 instead of straight burning the fuel for more features :p

I'll get around to profiling #125 .
This PR is going to take some time, and a lot of double checking. Hopefully by further unifying common functionality across track types we can gain back some bytes.

@yatli do you feel like adding the new compressed lock format to your todo list?
Ideally the code is to be portable between locks, notes, conditional data types.
We need some primitives like set_lock, get_lock, that should be optimized for the shared buffer.

@yatli do you feel like adding the new compressed lock format to your todo list?
Ideally the code is to be portable between locks, notes, conditional data types.
We need some primitives like set_lock, get_lock, that should be optimized for the shared buffer.

Can do!

We can also probably find a time to integrate https://github.com/jmamma/MIDICtrl20_MegaCommand/pull/5

We can also probably find a time to integrate #5

Yes.

Do we have any mechanism in C++ for dynamic typing.
For example
GridTrack empty_track;
MDTrack* md_track_temp = &empty_track;
A4Track* a4_track_temp = &empty_track;

//Curently we do this;
for (uint8_t n = 0; n < NUM_TRACKS; n++) {
  empty_track.load_slot_from_grid(n)
  if (empty_track.active == MD_TRACK_TYPE) {
  md_track->load_seq_data();
 }
else {
  a4_track->load_seq_data();
}

//I would like to do this, where load_seq_data() automatically calls the correct child's class function

for (uint8_t n = 0; n < NUM_TRACKS; n++) {
  empty_track.load_slot_from_grid(n)
  empty_track.load_seq_data();
}

Suggestions for New Project format.
Was thinking of creating a /Projects/ directory.
And then each project has its own directory.
/Project/NewProject/main.mcl  <- possibly not needed if we do auto grid detect using directory scanning?
/Project/NewProject/grid1.mcl
/Project/NewProject/grid2.mcl

What's the current status? Like, do I have to back up something?
I'm planning to write a GUI for accessing the SD card over USB.

Current status is:

I'm still hacking away at what needs changing. The above commits are just 'rough drafts' until I have a better grasp of how all the new data structures will interact.
Right now:
Need to create the multi grid system:

MDTrack layout no longer contains master FX settings, these will be saved as FX tracks on grid 2.
Ext/A4 Tracks should no longer be saved at position 16 + n. they should be saved in grid 2.

Multi grid system requires re-defining the Project header/layout and updating the new_project(); load_project() routines appropriately.

What's the current status? Like, do I have to back up something?
I'm planning to write a GUI for accessing the SD card over USB.

are you creating a serial driver?

No, just a COM proxy and a protocol (MegaCom!)

template <class C, int N, class T> T CRingBuffer<C, N, T>::size() volatile {
  if (wr >= rd) {
    return wr - rd;
  } else {
    return 256 << (sizeof(T) - 1) - rd + wr;
  }
}

why not len - rd + wr?

72   /** Returns the number of elements in the ring buffer. **/
73   T size() volatile;
i think this line is supposed to compensate for Type overflow. but it (256 << sizeof(T) - 1) looks wrong to me.

7482eb1 attempts to capture new project spec.
Projects/projectname/projectname.mcl <- contains only header information and mcl_cfg.
Projects/projectname/projectname.0 <- grid 0
Projects/projectname/projectname.1 <- grid 1

Slowly piecing this back together.
New project creation and loading is working.
I've added a "grid select" option to the slot menu.
However writing to the new grid files is currently broken.

Seemed to have reached some nesting limitations of the stack. When reading slots from Grid, in GUI event, MCL would crash.
Moving some grid.read/write functions to inline solves this.

Nesting limitation? Stack overflow?

I think so. The last commit didn't resolve it. I thought it was fixed, but the problem is still there.
614 void GridPage::apply_slot_changes() {
615   show_slot_menu = false;
616   encoders[0] = &param1;
617   encoders[1] = &param2;
618
619   uint8_t width;
620   uint8_t height;
621
622   EmptyTrack temp_track;
623   MDTrack *md_track = (MDTrack *)&temp_track;
624   A4Track *a4_track = (A4Track *)&temp_track;
625   ExtTrack *ext_track = (ExtTrack *)&temp_track;
626
627   GridTrack temp_slot;
628   temp_slot.load_from_grid(getCol(), getRow());

Line 628 is causing the reboot.

I think so. The last commit didn't resolve it. I thought it was fixed, but the problem is still there.
614 void GridPage::apply_slot_changes() {
615   show_slot_menu = false;
616   encoders[0] = &param1;
617   encoders[1] = &param2;
618
619   uint8_t width;
620   uint8_t height;
621
622   EmptyTrack temp_track;
623   MDTrack *md_track = (MDTrack *)&temp_track;
624   A4Track *a4_track = (A4Track *)&temp_track;
625   ExtTrack *ext_track = (ExtTrack *)&temp_track;
626
627   GridTrack temp_slot;
628   temp_slot.load_from_grid(getCol(), getRow());

Line 628 is causing the reboot.

 4 bool GridTrack::load_from_grid(uint8_t column, uint16_t row) {
  5
  6   bool ret;
  7
  8   uint32_t len = (sizeof(GridTrack));
  9
 10   DEBUG_PRINTLN("reading grid track");
 11   DEBUG_DUMP(len);
 12   uint16_t track_size = get_track_size();
 13   DEBUG_DUMP(track_size);
 14   delay(500);
 15   ret = proj.read_grid((uint8_t *)this, len, column, row);
 16   if (!ret) {
 17     DEBUG_PRINTLN("read failed");
 18     return false;
 19   }
 20   DEBUG_DUMP("read okay");
 21   len = get_track_size() - len;

len = 7
col = 0
row = 0
"read okay" = read okay
len = 8642

Somehow the len value becomes corrupt...
len = get_track_size() - len;
should == 0;

why is track size dump missing?

Copy pasta.
len = 7
track_size = 7

sigh. software can be so demoralising.

Might not be a nesting issue.
6 void MCL::setup() {
....
 85   while (1) {
 86           slot.load_from_grid(0,0);
 87           delay(500);
 88   }

Causes immediate lockup.

So we are able to use DEBUG_DUMP(SP) to print the position of the Stack Pointer.
Stack starts at memory address: 0x21FF = 8703
func_call: bool Project::read_grid(void*, size_t, uint8_t, uint16_t, uint8_t)
SP = 8689
grid = 0
len = 7
func_call: bool Grid::read(void*, size_t, uint8_t, uint16_t)
len = 7
SP = 8689
func_call: bool Grid::read(void*, size_t)
len = 7
func_call: bool MCLSd::read_data(void*, size_t, FatFile*)
SP = 8675
len = 23186

As long as SP is above 0x200 we're fine, which is clearly the case above.

Okay, so the problem is not len changing.
The problem is that get_track_size(); changes after the read.
get_track_size() = 7
func_call: bool Project::read_grid(void*, size_t, uint8_t, uint16_t, uint8_t)
get_track_size() = 23301
--
And I suspect it's because the read is overwriting the VirtualTable pointer of GridTask.
https://www.learncpp.com/cpp-tutorial/125-the-virtual-table/

Need to work out if getSize() includes the virtual table pointer. if so, we will have to adjust getSize(TrackClass) accordingly.

sizeof(GridTrack) is 7 bytes.
 26 class GridTrack {
 27 public:
 28   uint8_t active = EMPTY_TRACK_TYPE;
 29   GridChain chain;

sizeof(GridChain) = 4 bytes.
So the virtual table pointer is 2 bytes
5 + 2 = 7

Great work! And I think I understand what's happening here.
Serialized virtual class instances bring recalled (and things went wrong)
A potential problem is that the objects recalled in this way won't persist after code changes, where the functions and the vtable could have been relocated.
Are you using virtual functions to probe track types?

If so, we need to manually call the constructor to fix the vtable.

Yeah you're right.
After some consideration, I think using virtual functions here is not the right approach and adds additional complexity.

I had to re-check how things were done in Master branch.
We have base class GridTrack and then derived classes MDTrack, ExtTrack etc..
MDTrack has methods that override the base class, but they're not 'virtual'.
For example
class GridTrack {
bool store_in_grid()
bool load_from_grid()
}
class MDTrack : public DevTrack {
bool store_in_grid()
bool load_from_grid()
}
We then re-interpret the pointer given on the situation.
GridTrack temp_track;
MDTrack md_track = (MDTrack) &temp_track
temp_track->load_from_grid();
https://isocpp.org/wiki/faq/strange-inheritance#redefining-nonvirtuals
--
My alternative to avoid using virtuals, is to use the same approach.
Thoughts?

I think ultimately the problem I'm trying to solve is.

Load a track from any position in the grid, without knowing the track type in advance.

Perform some action on the track type, by dynamically casting to the appropriate class.

Requires knowing the size of the object to load. This is determined by reading the 'active' (TRACK_TYPE) value and then using get_track_size() to read the appropriate track length.

Is currently done by using switch(active) statements in various functions, to reinterpret cast the base class pointer to the correct class.

--
Perhaps, if we use virtual/vtable, and as you suggest call the appropriate constructor after load(), our objects will be dynamically typed?
Thus solving all of my problems?

Something like this:

class GridTrack {
public:
  static GridTrack* construct(void* blob) {
    switch(active) {
      case TRACK_TYPE_MD:
        // call MD default ctor with placement-new
        ::new(blob) MD_TRACK();
    }
  }
};

What’s the meaning of, Warning: Derived::f(char) hides Base::f(double)? ¶ Δ
It means you’re going to die.

C++ brogrammers heh.

I can implement that when I get home tonight.

https://www.nongnu.org/avr-libc/user-manual/FAQ.html#faq_cplusplus
The operators new and delete are not implemented, attempting to use them will cause the linker to complain about undefined external references. (This could perhaps be fixed.)
Not sure if this information is up to date?

Probably outdated, because it also claims that most standard libs are missing, but I actually used a few back then.
Also, placement new is different from normal new because it doesn't allocate memory, just calls the ctor. If that's not supported, classes won't be supported at all :D

Probably outdated, because it also claims that most standard libs are missing, but I actually used a few back then.
Also, placement new is different from normal new because it doesn't allocate memory, just calls the ctor. If that's not supported, classes won't be supported at all :D

Cool. Do you want to give it a try later tonight?
Roll back to and apply changes there.
1c28e51ae31c5a89d448095d6e38444b5420815a

Got it.
Back home after a long drive now.

https://github.com/jmamma/MIDICtrl20_MegaCommand/pull/129

After the merge, slot update is not working.
615 void GridPage::apply_slot_changes() {
616   show_slot_menu = false;
617   encoders[0] = &param1;
618   encoders[1] = &param2;
619
620   uint8_t width;
621   uint8_t height;
622
623   EmptyTrack temp_track;
624   MDTrack *md_track = (MDTrack *)&temp_track;
625   A4Track *a4_track = (A4Track *)&temp_track;
626   ExtTrack *ext_track = (ExtTrack *)&temp_track;
627
628   GridTrack temp_slot;
629   temp_slot.load_from_grid(getCol(), getRow(), false);
...
..
656
657   if (slot_copy + slot_paste + slot_clear == 0) {
658     if ((temp_slot.chain.row != slot.chain.row) ||
659         (temp_slot.chain.loops != slot.chain.loops)) {
660       slot_update = 1;
661     }

I did have this tested and working prior to merge.
So I suspect something is up with the load.
 temp_slot.load_from_grid(getCol(), getRow(), false);

False alarm. bad compile/link on my end.
Working again.

@yatli I need you to take a look at
void MCLActions::store_tracks_in_mem(int column, int row, uint8_t merge) {
  DEBUG_PRINT_FN();

  EmptyTrack empty_track;

  MDTrack *md_track = (MDTrack *)&empty_track;
#ifdef EXT_TRACKS
  A4Track *a4_track = (A4Track *)&empty_track;
  ExtTrack *ext_track = (ExtTrack *)&empty_track;
#endif
...
...

  for (i = 0; i < max_notes; i++) {
    if (note_interface.notes[i] == 3) {
...
...
      if (i < NUM_MD_TRACKS) {
        md_track->store_track_in_grid(i, grid_page.getRow(), merge, true);
        row_headers[grid_num].update_model(i, md_track->machine.model,
                                           MD_TRACK_TYPE);
      }
#ifdef EXT_TRACKS
      else {

        uint8_t track_num = i - NUM_MD_TRACKS;
        if (Analog4.connected) {
          DEBUG_PRINTLN("a4 get sound");
          Analog4.getBlockingSoundX(track_num);
          a4_track->sound.fromSysex(Analog4.midi);
        }
        a4_track->store_track_in_grid(track_num, grid_page.getRow(), true);
        row_headers[grid_num].update_model(track_num, track_num, A4_TRACK_TYPE);
      }
```

After the MD tracks are stored, the loop moves on to the A4/Ext tracks.

As  *md_track and *a4_track are both referencing the same object, a4_track->store_track_in_grid ends up incorrectly calling the MDTrack::store_track_in_grid.

Actually the DEBUG output is rather strange
13 22modelfunc_call: bool MDTrack::store_track_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: float MDMachine::normalize_level()
486
grid = 0
chain.length = 16
Track stored in grid
14 22modelfunc_call: bool MDTrack::store_track_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: float MDMachine::normalize_level()
486
grid = 0
chain.length = 16
Track stored in grid
15 22modela4 get sound
func_call: void MDSeqTrack::merge_from_md(uint8_t, MDPattern*)
func_call: float MDMachine::normalize_level()
486
grid = 1
chain.length = 16
Track stored in grid
26214416 22modela4 get sound
func_call: void MDSeqTrack::merge_from_md(uint8_t, MDPattern*)
func_call: float MDMachine::normalize_level()
486
grid = 1
chain.length = 16
Track stored in grid
26214417 22modela4 get sound
func_call: void MDSeqTrack::merge_from_md(uint8_t, MDPattern*)
func_call: float MDMachine::normalize_level()
486
grid = 1
chain.length = 16
Track stored in grid
26214418 22modela4 get sound
func_call: void MDSeqTrack::merge_from_md(uint8_t, MDPattern*)
func_call: float MDMachine::normalize_level()
486

bool MDTrack::store_track_in_grid(uint8_t, uint16_t, uint8_t, bool) <- doesnt print for the A4Track, but the sub routines
func_call: void MDSeqTrack::merge_from_md(uint8_t, MDPattern*)
func_call: float MDMachine::normalize_level()

do ?

Hmm maybe something is wrong with the new A4Track definition.
🤷

MCLMemory.h
36 #define A4_TRACK_LEN (sizeof(GridTrack) + sizeof(ExtSeqTrackData) + sizeof(A4Sound))
45 #define EMPTY_TRACK_LEN A4_TRACK_LEN
is probably no longer correct. ?

I don't see how the data in the EmptyTrack is loaded? There's just something that fills the GridTrack header..

MCLMemory.h
36 #define A4_TRACK_LEN (sizeof(GridTrack) + sizeof(ExtSeqTrackData) + sizeof(A4Sound))
45 #define EMPTY_TRACK_LEN A4_TRACK_LEN
is probably no longer correct. ?

Probably fine, as long as A4Track doesn't do the dreadful diamond inheritance.

store_track_in_grid for A4Track is virtual 😱😱😱

Now megacommand is hard rebooting on save.
--
store_track_in_grid and store_in_grid are supposed to be the same family.
I think ideally we make them virtual, to allow for some dynamic typing ?

Now I see we have different parameters for the parent and children.
ie:
./GridTrack.h:  bool store_in_grid(uint8_t column, uint16_t row, bool data = false);
./A4Track.h:  bool store_(track)_in_grid(uint8_t tracknumber, uint16_t row, bool online = false);

yeah, I left a few comments wrt this.

All in all, if we want to call a virtual function, we have to properly initialize the object.

All in all, if we want to call a virtual function, we have to properly initialize the object.

Yeah i had a quick play before.
wrap
bool init_track_type(uint8_t active) {
 15   switch (active) {
 16   case A4_TRACK_TYPE_270:
 17   case MD_TRACK_TYPE_270:
 18   case EXT_TRACK_TYPE_270:
 19     if (!data) {
 20       // no space for track upgrade
 21       return false;
 22     } else {
 23       // TODO upgrade right here
 24       return true;
 25     }
 26     break;
 27   case EMPTY_TRACK_TYPE:
 28     ::new(this) EmptyTrack;
 29     break;
...
}

Then

Empty empy_track;
empty_track.init_track_type(A4_TRACK_TYPE);
empty_track.store_in_mem...

Not sure how many fundamental laws of the universe I was breaking by doing this...

Regarding
store_in_grid(uint8_t column, uint16_t row, bool data = false);
I originally got away without using the data variable.
By overriding get_track_size() for GridTrack->store_in_grid. (Using the intermediate DeviceTrack class)
The parameters of the parent and child store_track_in_grid, would lineup this way.

Empty empy_track;
empty_track.init_track_type(A4_TRACK_TYPE);
empty_track.store_in_mem...

That's it.

What's the difference between store_in_grid and store_track_in_grid (and their load counterparts)?
If it's additive (base functionality + extra device logic) we can go with the augmentation pattern:
virtual void SomeDeviceTrack::store_in_grid(....)
{
  GridTrack::store_in_grid(...);
  device_logic();
}

If it's additive (base functionality + extra device logic)

Yes this is it basically.
virtual void SomeDeviceTrack::store_in_grid(....)
{
  device_logic();
  GridTrack::store_in_grid(...);
}

@yatli regarding a40bc5a
I've reverted back to an earlier solution using middle tier DeviceTrack class.
The idea is that GridTrack->load_from_grid GridTrack->save_to_grid will implicitly only ever load/store the header without data.
As soon as you move down the hierarchy and call load_from_grid/save_to_grid from a DeviceTrack type. MDTrack, A4Track etc... the full data payload will be loaded/saved.

Makes sense. We can get rid of the data parameter.

@yatli need an extra pair of 👀
a40bc5a
md_track->store_in_grid(..) is not working.
 42 void MCLActions::store_tracks_in_mem(int column, int row, uint8_t merge) {
...
 44
 45   EmptyTrack empty_track;
 46
 47   MDTrack *md_track = (MDTrack *)&empty_track;
...
142       if (i < NUM_MD_TRACKS) {
143         md_track->store_in_grid(i, grid_page.getRow(), merge);
144         row_headers[grid_num].update_model(i, md_track->machine.model,
145                                            MD_TRACK_TYPE);
146       }

debug before and after write machine.model = garbage.
func_call: bool MDTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
491
machine.model =<PHONE_NUMBER>
What have I missed?

This is the part I didn't understand.
Nothing is loading data into empty track, and it's not initialized.

d'oh problem was here
md_track->store_in_grid(i, grid_page.getRow(), merge, true);
^

Also
MDTrack *md_track = (MDTrack *)&empty_track;
is re-interpret cast.
currently store_in_grid and load_from_grid are not virtual.
i will make the switch now.

so it looks like we've achieved dynamic typing.
I didnt think this would work... or be possible. 🤯

When you have dark matter, you don't care about the universal laws anymore (Arnold face

hmm spoke too soon.

something is badly broken after
e9af49f

have a headache. leaving this for tonight.

@yatli need you think of a dynamic memory solution for BANK1 regions store_in_mem/load_from_mem
We will need to cache different track sizes, and currently we are limited to two regions of fixed size + fixed allocations..
For example we willl have MDTrack + A4Track + ExtTrack + FXTracks + MNMTrack ??

MDTrack + A4Track + ExtTrack + FXTracks + MNMTrack

We aren't going to have them all in one row, right :)

 15 class DeviceTrack : public GridTrack {
 16
 17 public:
 18   uint8_t active = EMPTY_TRACK_TYPE;
 19
 20   GridChain chain;
 21   //  bool get_track_from_sysex(int tracknumber, uint8_t column);
 22   //  void place_track_in_sysex(int tracknumber, uint8_t column);
 23   virtual bool store_in_grid(uint8_t column, uint16_t row, uint8_t merge = 0,
 24                            bool online = false)

making store_in_grid virtual, is leading to undefined behaviour for
138         empty_track.init_track_type(MD_TRACK_TYPE);
139         empty_track.store_in_grid(i, grid_page.getRow(), merge, true);
program gets stuck in infinite loop calling:
func_call: bool GridTrack::init_track_type(uint8_t)

@yatli what have I messed up here?

Let me switch to this branch.

interestingly if I try and move
23   virtual bool store_in_grid(uint8_t column, uint16_t row, uint8_t merge = 0,
24                            bool online = false)
up in to GridTrack.h
(which now has store_in_grid(..,..) and store_in_grid(..,..,..,..,)
we have
/Applications/Arduino.app/Contents/Java/hardware/valence/avr/cores/megacommand/MCL/Grid.cpp:136:41: error: call of overloaded 'store_in_grid(int16_t&, int16_t&)' is ambiguous
empty_track.store_in_grid(d_col, d_row);

could you explain what does MCLActions::store_tracks_in_mem do? I'm totally lost in this.
The empty track doesn't have the data (not even the right active field), and it's being written into the grid?

/Applications/Arduino.app/Contents/Java/hardware/valence/avr/cores/megacommand/MCL/Grid.cpp:136:41: error: call of overloaded 'store_in_grid(int16_t&, int16_t&)' is ambiguous empty_track.store_in_grid(d_col, d_row);

That's because the DeviceTrack version has default parameters.
Oh.
program gets stuck in infinite loop and can be seen calling:
Do you mean it's calling itself again and again?

That's because the DeviceTrack version has default parameters.

could you explain what does MCLActions::store_tracks_in_mem do? I'm totally lost in this.
The empty track doesn't have the data (not even the right active field), and it's being written into the grid?

138 empty_track.init_track_type(MD_TRACK_TYPE);
139 empty_track.store_in_grid(i, grid_page.getRow(), merge, true);

---->
136 bool MDTrack::store_in_grid(uint8_t tracknumber, uint16_t row,
137                                   uint8_t merge,
138                                   bool online) {
139   active = MD_TRACK_TYPE;  <---- SET ACTIVE
140
141   bool ret;
142   int b = 0;
143   DEBUG_PRINT_FN();
144   uint32_t len;
145
146   if (tracknumber != 255 && online == true) {
147     get_machine_from_kit(tracknumber);   <--- GET DATA FROM SYSEX

/Applications/Arduino.app/Contents/Java/hardware/valence/avr/cores/megacommand/MCL/Grid.cpp:136:41: error: call of overloaded 'store_in_grid(int16_t&, int16_t&)' is ambiguous empty_track.store_in_grid(d_col, d_row);

Ah yeah. okay.

That's because the DeviceTrack version has default parameters.
Oh.
program gets stuck in infinite loop and can be seen calling:
Do you mean it's calling itself again and again?

Debug is spammed with
func_call: bool GridTrack::init_track_type(uint8_t)

for t->infinity

That's because the DeviceTrack version has default parameters.

could you explain what does MCLActions::store_tracks_in_mem do? I'm totally lost in this.
The empty track doesn't have the data (not even the right active field), and it's being written into the grid?

138 empty_track.init_track_type(MD_TRACK_TYPE);
139 empty_track.store_in_grid(i, grid_page.getRow(), merge, true);

---->
136 bool MDTrack::store_in_grid(uint8_t tracknumber, uint16_t row,
137                                   uint8_t merge,
138                                   bool online) {
139   active = MD_TRACK_TYPE;  <---- SET ACTIVE
140
141   bool ret;
142   int b = 0;
143   DEBUG_PRINT_FN();
144   uint32_t len;
145
146   if (tracknumber != 255 && online == true) {
147     get_machine_from_kit(tracknumber);   <--- GET DATA FROM SYSEX

The function purpose is to save tracks to grid. It's called from the save page.

The idea is, that by setting the track type.
empty_track.init_track_type(MD_TRACK_TYPE);
empty_track now inherits the virtual table which has the correct reference for
136 bool MDTrack::store_in_grid(uint8_t tracknumber, uint16_t row,
137                                   uint8_t merge,
138                                   bool online) {

I don't understand... How does store_in_grid in turn calls GridTrack::init_track_type?
It's referenced only from GridTrack::load_from_grid

I don't understand... How does store_in_grid in turn calls GridTrack::init_track_type?

see DeviceTrack.cpp
  5 bool DeviceTrack::init_track_type(uint8_t track_type) {
  6   DEBUG_PRINT_FN();
  7   switch (track_type) {

EmptyTrack  : public DeviceTrack
so
empty_track.init_track_type(MD_TRACK_TYPE) -> DeviceTrack::init_track_type(MD_TRACK_TYPE);

Yeah, but GridTrack::init_track_type?

Yeah, but GridTrack::init_track_type?

If you're referring to the debug output.
it shouldnt be calling this function.

func_call: void MCLActions::store_tracks_in_mem(int, int, uint8_t)
kit okay
func_call: bool Project::read_grid_row_header(GridRowHeader*, uint16_t, uint8_t)
grid = 0
active = 0
func_call: bool Project::read_grid_row_header(GridRowHeader*, uint16_t, uint8_t)
grid = 0
active = 0
func_call: bool GridTrack::init_track_type(uint8_t)
7
active = 0
func_call: bool GridTrack::init_track_type(uint8_t)
7

...
..
..
..

Currently init_track_type is not virtual... it's be overridden in DeviceTrack.
perhaps this is the issue shrug

line 129,         grid_track.load_from_grid(i, row);
could this be the entry to GridTrack::init_track_type?

line 129,  grid_track.load_from_grid(i, row);
could this be the entry to GridTrack::init_track_type?

ah yeah. that's probably the call. i dont know why it's being spammed though.

It reports len=7 which is correct for GridTrack. So the vtable at GridTrack::load_from_grid should be ok.

So incorrectly referencing GridTrack now when it should be MDTrack.

The inheritance diverged.
GridTrack -> DeviceTrack -> EmptyTrack
                  \
                   \----> MDTrack

EmptyTrack has different vtable structure than MDTrack.
So you cannot conduct MDTrack initialization on EmptyTrack, and later on a EmptyTrack object, call a virtual function.

How do the VTables differ exactly?

#include <iostream>
#include <new>
using namespace std;

class GridTrack {
  public:
    virtual void foo() { cout << "GridTrack foo" << endl; }
};

class DeviceTrack: public GridTrack {
  public:
    virtual void foo(int x) { cout << "DeviceTrack foo" << endl; GridTrack::foo(); }
};

class EmptyTrack: public DeviceTrack {
  public:
     void foo() { cout << "EmptyTrack foo" << endl;}
};

class MDTrack: public DeviceTrack {
  public:
    void foo() { cout << "MDTrack foo" << endl; }
};

int main()
{
  EmptyTrack empty_track;
  ::new(&empty_track) MDTrack;
  empty_track.foo();

  return 0;
}

Guess what it does? "EmptyTrack foo", not "MDTrack foo"

Now:
int main()
{
  EmptyTrack empty_track;
  ::new(&empty_track) MDTrack;
  GridTrack* ptrack = &empty_track;
  empty_track.foo();
  ptrack->foo();

  return 0;
}

EmptyTrack foo
MDTrack foo

I did the ptrack thing for a reason. :p

thanks!
let me look at this again in the morning
with a fresh 🧠

You are becoming a C++ veteran.

So we need to reinterpret cast empty_track to GridTrack, to get an interpretation of the vtable that will work with child classes?

Will
DeviceTrack* ptrack = &empty_track;
work? or do we need to go back to the parent?

DeviceTrack* will do.

Oh and, this:
empty_track.init_track_type(MD_TRACK_TYPE); will call into GridTrack::init_track_type.

Oh and, this:
empty_track.init_track_type(MD_TRACK_TYPE); will call into GridTrack::init_track_type.

why?

sorry, should be DeviceTrack::init_track_type.

Feel free to fix up the code whilst i'm sleeping 😀

Alright! Need some help, wait

What's the safe way to play with this branch?
I don't have screwdriver to get to the SD card, so I haven't backed up my stuff..

oh. yeah do that. i'm using a separate megacommand for this branch.
The MegaCom thing is too slow for the task(
need more turbo.

need more turbo.

Actually I designed it to work like this. Only 512 bytes will be transmitted per GUI loop, to save bandwidth and processor for other tasks.

What's the safe way to play with this branch?
As soon as you boot up it will create the new project directory
/Projects/
and then create subfolders for each new project.
So you wont risk overwriting anything.
I can't make any guarantees here. i'd recommend backup.

need more turbo.

Actually I designed it to work like this. Only 512 bytes will be transmitted per GUI loop, to save bandwidth and processor for other tasks.

time share system :D.

need more turbo.

On second thought, though....
250000bps = 31.25KB/s
Currently MegaCom file transfer has 20KB/s with a 5ms budget.
Let me try crank it up and see what will happen..

No deadloop.
Made the debug more verbose
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool DeviceTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0
active = 0
func_call: bool GridTrack::init_track_type(uint8_t)
7
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool DeviceTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0
active = 0
func_call: bool GridTrack::init_track_type(uint8_t)
7
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool DeviceTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0
active = 0
func_call: bool GridTrack::init_track_type(uint8_t)
7
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool DeviceTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0
active = 0

So
 15 class DeviceTrack : public GridTrack {
 16
 17 public:
 18   uint8_t active = EMPTY_TRACK_TYPE;
 19
 20   GridChain chain;
 21   //  bool get_track_from_sysex(int tracknumber, uint8_t column);
 22   //  void place_track_in_sysex(int tracknumber, uint8_t column);
 23   virtual bool store_in_grid(uint8_t column, uint16_t row, uint8_t merge = 0,
 24                            bool online = false) {
 25      DEBUG_PRINT_FN();
 26      GridTrack::store_in_grid(column,row);
 27   }

Is being called, instead of the virtual of the specified track type

I finished my backup.

Feels like we've hit a dead end.
Maybe we can create our own virtual function table?

I finished my backup.

That was quick!
--
I think we've hit a dead end. compiler is selecting DeviceTrack::store_track_in_grid over virtual..

I got:
func_call: void MCLActions::store_tracks_in_mem(int, int, uint8_t)
kit okay
func_call: bool Project::read_grid_row_header(GridRowHeader*, uint16_t, uint8_t)
grid = 0
func_call: bool Project::read_grid_row_header(GridRowHeader*, uint16_t, uint8_t)
grid = 0
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0

Let's try make DeviceTrack::store_in_grid pure virtual.

It's likely init_track_type didn't work.

Let's try make DeviceTrack::store_in_grid pure virtual.

tried that. remove definition in DeviceTrack and just have the virtual declaration.
linker fails to find reference to store_track_in_mem

Pure virtual:
virtual bool store_in_grid(uint8_t column, uint16_t row, uint8_t merge = 0, bool online = false)
                   = 0;

It then turns DeviceTrack into an abstract class.
I then moved the implementation (calling GridTrack::save_to_grid) to EmptyTrack.
Then the empty track method is called despite init_track_type:
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool EmptyTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0

Pure virtual:
virtual bool store_in_grid(uint8_t column, uint16_t row, uint8_t merge = 0, bool online = false)
                   = 0;  // here

DIdn't know about this.

It then turns DeviceTrack into an abstract class.
I then moved the implementation (calling GridTrack::save_to_grid) to EmptyTrack.
Then the empty track method is called despite init_track_type:
func_call: bool DeviceTrack::init_track_type(uint8_t)
func_call: virtual bool EmptyTrack::store_in_grid(uint8_t, uint16_t, uint8_t, bool)
func_call: bool GridTrack::store_in_grid(uint8_t, uint16_t)
grid = 0

😞

Let's verify whether the MDTrack ctor is called:
class MDTrack : public DeviceTrack {
public:
MDSeqTrackData seq_data;
MDMachine machine;
MDTrack() : DeviceTrack() {
DEBUG_PRINT_FN();
}

It is called.

The compiler cached the vtable...

@jmamma done.
98264bd

😮
So how does returning a pointer resolve the vtable caching?

The compiler doesn't know how the pointer is obtained. So there's no way to cache the vtable ahead of time.

template bool is() { return dynamic_cast<T*>(this) != nullptr; }

Saving MD tracks works now.
opening slot menu crashes.

Opening slot menu, on a slot that has  MD track saved crashes.
If it is empty, it works.
So  slot.load_from_grid(getCol(), getRow()); must be incorrectly loading MDTrack worth of data now...

bool GridTrack::init_track_type(uint8_t track_type) {
   ::new(this) GridTrack;
   return true;
}

this was needed to make GridTrack->load_from_grid only load sizeo(GridTrack);
Can we add it back or is there a better fix?

Can we add it back or is there a better fix?

Hard-coded sizeof(GridTrack) in the base implementation.

47a1b43 is that what you had in mind?

ha. it's already hard-coded in earlier commits -- anything that instantiates as GridTrack will only load the header.
Only DeviceTrack will get interesting

ha. it's already hard-coded in earlier commits -- anything that instantiates as GridTrack will only load the header.
?

I must have missed it.

/Applications/Arduino.app/Contents/Java/hardware/valence/avr/cores/megacommand/MCL/GridTask.cpp:69:38: error: no matching function for call to 'MDTrack::load_from_mem(uint8_t&)'
md_track->load_from_mem(n);
^ compile error

To access GridTrack header only:
GridTrack grid_track;
grid_track.load_from_grid(...); // ok, no data

To access the data:
EmptyTrack empty_track;
auto md_track = empty_track.load_from_grid<MDTrack>(...); // full MDTrack with data. null if type mismatch or load failure

_dynamik_kast
tha h4x are real.

I tried dynamic_cast but the compiler is not happy (-fno-rtti).
So let's just fake it.

this is my favourate:
        if (ext_track->is<A4Track>()) {
          if ((mcl_actions.a4_latency > 0) &&
              (mcl_actions.send_machine[n] == 0)) {
            DEBUG_PRINTLN("here");
            DEBUG_PRINTLN("send a4 sound");
            ext_track->as<A4Track>()->sound.toSysex();
          }
        }

almost like a diagram.

my brain is on the verge of collapsing.

Take a look at MCLActions.cpp or GridTask.cpp, it's actually much cleaner than before.

save/load are ok now.
chain playback not working.

so many changes. so much confusion.

alright alright I'll help you to go through them. just throw me questions.

  5 bool GridTrack::load_from_grid(uint8_t column, uint16_t row) {
  6
  7   if (!proj.read_grid(this, sizeof(GridTrack), column, row)) {
  8     DEBUG_PRINTLN("read failed");
  9     return false;
 10   }
 11
 12   auto tmp = this->active;
 13   ::new(this) GridTrack;
 14   this->active = tmp;
 15

why are lines 12->14 needed, if we now override this method in DeviceTrack ?

perhaps this view will give you a better idea: https://github.com/jmamma/MIDICtrl20_MegaCommand/pull/127/files/df60c6ea23a8cdf215f276d6d8419eed03fe55cb..cd2bdac014f59ad1dd2642fd08c405373abd3786

why are lines 12->14 needed, if we now override this method in DeviceTrack ?
because the GridTrack ctor will initialize active to EMPTY_TRACK_TYPE.

oh, that's a good one.
Actually I have to do this for DeviceTrack also.

Why do we need to reconstruct it
::new(this) GridTrack;
?

Maybe it's not necessary.
We may even consider move all the virtual functions to DeviceTrack.

The reconstruct of base type will guarantee that any accidental call to get_track_size() correctly return just sizeof(GridTrack)
i'm trying to think of a scenario where that would happen.
and then i go layers deep in confusion.

We have laid out the condition that
GridTrack->load_from_grid only reads GridTrack size of data.
In what scenario would this fail?
If we recast the object?

In what scenario would this fail?

Um, SD card failure?

``The reconstruct of base type will guarantee that any accidental call to get_track_size() correctly return just sizeof(GridTrack)```
can you give me an example of an accidental call?

grid_track.load_from_grid(i, row);
grid_track.get_track_size(); /// would crash if not initialized properly

65   virtual uint16_t get_track_size() { return sizeof(GridTrack); }
I dont see how the call to get_track_size would return anything other than ^

Yeah, especially if the compiler caches the vtable.. So probably not a big deal.

Yep.. and now moving on.
So to circumvent the caching of the vtable you made the following functions return pointers instead of bool.
48   DeviceTrack *init_track_type(uint8_t track_type);
50   DeviceTrack *load_from_grid(uint8_t column, uint16_t row);

Correct.

The compiler doesn't know how the pointer is obtained. So there's no way to cache the vtable ahead of time.
can you elaborate on that ?

If we do:
DeviceTrack* p = &empty_track;
.....
p->foo();

The compiler knows that p still points to empty_track.
The type of empty_track is EmptyTrack.
So it will decide to not look up the vtable, but directly call into EmptyTrack::foo().
But, if we have DeviceTrack* DeviceTrack::get()
Then the compiler doesn't know that the pointer returned still points to "this" (actually it cannot know, because we also return nullptr on failure), so the type inference chain is broken.

This is a gcc behavior. I tested with MSVC yesterday, no such clever trick.

Then the compiler doesn't know that the pointer returned still points to "this" (actually it cannot know, because we also return nullptr on failure), so the type inference chain is broken.

Right and you exploit this using the 'auto' keyword
auto pdevice_track = empty_track.init_track_type(MD_TRACK_TYPE);

Would it work without auto?

Would it work without auto?

That's orthogonal to the exploit.
You can also write DeviceTrack* pdevice_track = empty_track.init_track_type(MD_TRACK_TYPE);

Ok, now we reach the plateau of mildly-typed objects, of type "DeviceTrack".
When you simply do empty_track.init_track_type(...) or empty_track.load_from_grid(....), it will return DeviceTrack*.

The traditional way of telling the type still works:
pdevice_track->active == MD_TRACK_TYPE // ok

But now we have something more strongly-typed:
pdevice_track->is<MDTrack>()

ack.
moving on.
(I feel like a prosecutor in law proceedings interrogating the suspect)
You've  overloaded load_from_grid, using templates. This creates a version of load_from_grid for each track type?
50   DeviceTrack *load_from_grid(uint8_t column, uint16_t row);
 51   template <class T> T *load_from_grid(uint8_t col, uint16_t row) {
 52     auto *p = load_from_grid(col, row);
 53     if (!p) return nullptr;
 54     return _dynamik_kast<T>(p);
 55   }

Dynamic casting is used to verify that the load / type assignment is valid.

Correct.
The interrogation works!

EMPTY_TRACK_TYPE || p->active == 255
EXT_TRACK_TYPE || p->active == A4_TRACK_TYPE
mixing boolean with uint8_t ?

oh it's macro.
nevermind.

I have no further questions... at least for now.

MD saving is not working anymore. not sure which commit broke it.

MD Saving works when DEBUGMODE is enabled.
Stops working when debugmode is disabled.
Fresh compiles on each.

Also work noting that dynamik_kast is consuming around 2KB of progmem than my earlier compiles.

Also worth noting that dynamik_kast is consuming around 2KB of progmem than my earlier compiles.

Really? Didn't expect that.

Latest: Sketch uses 195478 bytes (76%) of program storage space. Maximum is 253952 bytes.
df60c6e: Sketch uses 194242 bytes (76%) of program storage space. Maximum is 253952 bytes.
I think it's a price worth paying.

I have SAVE working, but CHAIN not.

Chain mode is freezing or just not working?
Could be broken for other reasons. I haven't gone over the code yet.
--
Are you able to verify save is working when debug is disabled. Open save page then press shift2 to save all MD tracks. (leave your A4 off)

Chain mode is freezing or just not working?

It just reboots.
I believe 27751b8 fixed that.

Are you able to verify save is working when debug is disabled. Open save page then press shift2 to save all MD tracks. (leave your A4 off)

It saves 16 tracks to the grid. I can see correct models being displayed at the saved locations.
Chain doesn't crash now. Recalls the kit correctly. But doesn't playback the sequence, so cannot verify if SAVE actually writes the sequence data.

Playback is ok now.
The GridPage doesn't blink the active slots.

It saves 16 tracks to the grid. I can see correct models being displayed at the saved locations.
hmm that wasnt happening for me.
I have a bad migraine. (been lingering the last few days). I have to take the rest of the night off.
hands you the keys

grabs the key, but continues to use the Angle Grinder against the problem

16 md track, 6 a4 track, 256 file entries, buffers...
Adds up to address 0xFCB8.

burning out. I'm done for the night.

notes:

some wrong offsets in grid io. mostly idx > NUM_MD_TRACK and thrown directly to grid 2.
save all on grid 1 (no A4 attached) also saves data to grid2

@yatli grid 1 should display the full 16 tracks.
We are going to use those slots to store MD FX. currently the FXs are not being collected.
Unless you wanted to make a 3rd grid...

Also, saving MD tracks is stil not working for me with debug off.
The row name is updated, but the slot models are all -- -- -- -- -- --

Does save individual track work?

nope individual failing as well.
did you tweak any compile options on your end?

nope, just the usual build.ps1 -clean -upload

Interestingly, on my end the tracks get stored properly, and the kit can be chained, but the row name is gone

Justins-MacBook-Pro-4:megacommand jmammarella$ md5 main.hex
MD5 (main.hex) = 8952eda8746227d9d11c3beccaca9ee4
avrdude: writing flash (199756 bytes):

our builds are never bitwise identical... -_-"

137       if (i < NUM_MD_TRACKS) {
138         auto pdevice_track = empty_track.init_track_type(MD_TRACK_TYPE);
139         oled_display.clearDisplay();
140         if (pdevice_track->is<MDTrack>()) { oled_display.textbox("IS_MD 1",""); }
141         oled_display.display();
142         delay(500);
143         pdevice_track->store_in_grid(i, grid_page.getRow(), merge,
144                                                    true);
145         if (pdevice_track->is<MDTrack>()) { oled_display.textbox("IS_MD 2",""); }
146         oled_display.display();
147         row_headers[grid_num].update_model(i, pdevice_track->get_model(),
148                                            MD_TRACK_TYPE);
149       }

I tried a bit of live debugging.
On both DEBUG and no DEBUG.
"IS_MD 2" is displayed.
"IS_MD 1" is never displayed...

try dump the active field?

Interestingly, on my end the tracks get stored properly, and the kit can be chained, but the row name is gone

False alarm. I cleared the row names, so they row is "active" and the name is not pulled in from MD.

"IS_MD 2" is displayed.
"IS_MD 1" is never displayed...

Can confirm. Investigating...

      if (i < NUM_MD_TRACKS) {
        auto pdevice_track = empty_track.init_track_type(MD_TRACK_TYPE);
        oled_display.clearDisplay();
        char str[3];
        itoa(pdevice_track->active, str, 10);
        oled_display.textbox(str,str); <------- 00, 00
//if (pdevice_track->is<MDTrack>()) { oled_display.textbox("IS_MD 1",""); }
        oled_display.display();
        delay(500);
        pdevice_track->store_in_grid(i, grid_page.getRow(), merge,
                                                   true);
        itoa(pdevice_track->active, str, 10);
        oled_display.textbox(str,str); <--------- 44, 44
        // if (pdevice_track->is<MDTrack>()) { oled_display.textbox("IS_MD 2",""); }
        oled_display.display();
        delay(500);
        row_headers[grid_num].update_model(i, pdevice_track->get_model(),
                                           MD_TRACK_TYPE);
      }
#ifdef EXT_TRACK

should be working now @jmamma

nice! MD Tracks are saving now.
A4 tracks save is still broken, but this has been an issue for a while...

another problem: reboots after a few chain writes.

the reboot problem seems to be gone after
d853379

@jmamma step edit page, utiming knob value jitters between {-1, 1} (again). Any idea how to fix?

the reboot problem seems to be gone after
d853379

awesome. i suspected something like this.

@jmamma step edit page, utiming knob value jitters between {-1, 1} (again). Any idea how to fix?

yeah i noticed that a while back.
i'll take a look.

I suspect the problem lies here, because I refactored SEQ_SPEED_1x/2x... etc to be consistent amongst track types.
37 void SeqStepPage::config_encoders() {
 38   uint8_t timing_mid = mcl_seq.md_tracks[last_md_track].get_timing_mid();
 39   seq_param2.cur = timing_mid;
 40   seq_param2.old = timing_mid;
 41   seq_param2.max = timing_mid * 2 - 1;
 42 }

So maybe mcl_seq.md_tracks[last_md_track].speed; is not getting initialised correctly.

 37 void SeqStepPage::config_encoders() {
 38   uint8_t timing_mid = mcl_seq.md_tracks[last_md_track].get_timing_mid();
 39   DEBUG_DUMP(timing_mid);

hmm
timing_mid = 0
the default return should be 12.

next time we disable -fpermissive


[Bank Tags] Tiny tempor tag icon not centered

Describe the bug
Tempoross inventory icon is not centered and looks really off.
To Reproduce

Change icon on a tag tab
Search and select 'tiny tempor'
Note off-center icon

Expected behavior
Icon would be center
Screenshots

Environment (please complete the following information):

OS: Windows
RuneLite version: 1.7.3
Launcher version: 2.1.0

We don't control the centering of icons. If its centering is an issue, please report this to Jagex.


There seems no definition of Platform::free and so on for macOS.

my clang says "Undefined symbols". Does eMQTT5 support macOS?
Undefined symbols for architecture x86_64:
  "Platform::free(void*, bool)", referenced from:
      Network::Client::MQTTv5::connectTo(char const*, unsigned short, bool, unsigned short, bool, char const*, Protocol::MQTT::Common::DynamicBinDataView const*, Protocol::MQTT::V5::WillMessage*, Protocol::MQTT::V5::QualityOfServiceDelivery, bool, Protocol::MQTT::V5::Properties*) in libeMQTT5.a(MQTTClient.cpp.o)
      Network::Client::MQTTv5::Impl::Impl(char const*, Network::Client::MessageReceived*, Protocol::MQTT::Common::DynamicBinDataView const*) in libeMQTT5.a(MQTTClient.cpp.o)
      Protocol::MQTT::Common::DynamicString::readFrom(unsigned char const*, unsigned int) in libeMQTT5.a(MQTTClient.cpp.o)
      Network::Client::MQTTv5::Impl::~Impl() in libeMQTT5.a(MQTTClient.cpp.o)
  "Platform::malloc(unsigned long, bool)", referenced from:
      Network::Client::MQTTv5::connectTo(char const*, unsigned short, bool, unsigned short, bool, char const*, Protocol::MQTT::Common::DynamicBinDataView const*, Protocol::MQTT::V5::WillMessage*, Protocol::MQTT::V5::QualityOfServiceDelivery, bool, Protocol::MQTT::V5::Properties*) in libeMQTT5.a(MQTTClient.cpp.o)
      Network::Client::MQTTv5::Impl::Impl(char const*, Network::Client::MessageReceived*, Protocol::MQTT::Common::DynamicBinDataView const*) in libeMQTT5.a(MQTTClient.cpp.o)
  "Platform::realloc(void*, unsigned long)", referenced from:
      Protocol::MQTT::Common::DynamicString::readFrom(unsigned char const*, unsigned int) in libeMQTT5.a(MQTTClient.cpp.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I was developing on a Mac when I wrote this, so it should work. Can you send me your CMake's build configuration?

The definition of the 3 methods are in eMQTT5/tests/ClassPath/src/Posix.cpp.

Can you add a -D_POSIX to your CXX flags to see if it works ? I wonder if they removed the preprocessor macro recently.

Thank you for very quick reply! Now I'll try -D_POSIX.

I added -D_POSIX. but result was same.
I indicate eMQTT5 as a link target from my CMakeLists.txt. Is it correct? What is a correct target name?
Is it written in the document below? I can't read it because URL is collapse.

API Documentation
You'll find the client API documentation here.
https://blog.cyril.by/en/documentation/emqtt5

According to my cmake knowledge, it seems that only one source file is compiled for the target eMQTT5 which doesn't include eMQTT5/tests/ClassPath/src/Posix.cpp.

add_library(eMQTT5 src/Network/Clients/MQTTClient.cpp)

There seems no definition of Platform::free and so on for library eMQTT5. Excuse me, could you please tell me the proper way to write my cmake to link eMQTT5 as a library.
now i just write like:
TARGET_LINK_LIBRARIES(myProj
  eMQTT5
)

I've fixed the URL. The library shouldn't need platform code, only the test code does. Let me check...

I just use Network::Client::MQTTv5 as a library. I include #include <Network/Clients/MQTT.hpp> from my code.

Can you try with latest master ? It should have fixed the issue.

I tried but same. Only src/Network/Clients/MQTTClient.cpp is compiled for libeMQTT5.a
Is this your modification correct?
https://github.com/X-Ryl669/eMQTT5/blob/b197dacbaf1aa31df3eb5ef841b6c24744398dcf/lib/CMakeLists.txt#L3

Can you try to apply this patch on the latest master and try to build it (it should fail, please post the log):
diff --git a/lib/include/Platform/Platform.hpp b/lib/include/Platform/Platform.hpp
index 30d15eb..322e3e8 100644
--- a/lib/include/Platform/Platform.hpp
+++ b/lib/include/Platform/Platform.hpp
@@ -49,28 +49,27 @@ namespace Platform
     /** Get the current process name.
         This does not rely on remembering the argv[0] since this does not exists on Windows.
         This returns the name of executable used to run the process */
-    inline const char * getProcessName() {
+    inline const char * getProcessName()^M
     {
         static char * processName = NULL;
         if (!processName)
         {
-#ifdef _LINUX
+  #ifdef _LINUX^M
             FILE * f = fopen("/proc/self/cmdline", "r");
             if (f) {
                 char buffer[256];
                 processName = strdup(fgets(buffer, 256, f));
                 fclose(f);
             }
-#elif defined(_MAC)
+  #elif defined(_MAC)^M
             processName = strdup(getprogname());
-#else
+  #else^M
             return "program";
-#endif
+  #endif^M
         }
         return processName;
     }
-
-return "this_program"; } // Poor workaround to avoid so many difference between platforms
+#error here^M
 #else
     /** The simple malloc overload.
         If you need to use another allocator, you should define this method

:pray: Could you push it to a branch?

In the latest master, I've removed Posix.cpp file since it was useless (even for tests). Yet, it seems the compiler tries to build it anyway. It's strange.

Try the branch called "badStuff", run rm -rf build && mkdir build then rerun ccmake .. in the build directory and post output. It should fail building.

[  1%] Building CXX object common_libs/LoadSensor/common_libs/eMQTT5/lib/CMakeFiles/eMQTT5.dir/src/Network/Clients/MQTTClient.cpp.o
cd /Users/shinya.ohtani/unix/cloned/build/ThermoView_cli/common_libs/LoadSensor/common_libs/eMQTT5/lib && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DMQTTOnlyBSDSocket=1 -DMQTTUseTLS=0 -DMinimalFootPrint=1 -D_DEBUG=0 -I~/repos/external/eMQTT5/lib/include -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk -mmacosx-version-min=10.15 -std=c++11 -o CMakeFiles/eMQTT5.dir/src/Network/Clients/MQTTClient.cpp.o -c ~/repos/external/eMQTT5/lib/src/Network/Clients/MQTTClient.cpp
In file included from ~/repos/external/eMQTT5/lib/src/Network/Clients/MQTTClient.cpp:2:
In file included from ~/repos/external/eMQTT5/lib/src/Network/Clients/../../../include/Network/Clients/MQTT.hpp:5:
~/repos/external/eMQTT5/lib/src/Network/Clients/../../../include/Network/Clients/MQTTConfig.hpp:89:9: warning:
      Building eMQTT5 with flags: __Check__BSD [-W#pragma-messages]
#pragma message("Building eMQTT5 with flags: " CONF_AUTH CONF_DUMP CONF_VALID CONF_TLS CONF_SOCKET)
        ^
In file included from ~/repos/external/eMQTT5/lib/src/Network/Clients/MQTTClient.cpp:2:
In file included from ~/repos/external/eMQTT5/lib/src/Network/Clients/../../../include/Network/Clients/MQTT.hpp:8:
In file included from ~/repos/external/eMQTT5/lib/src/Network/Clients/../../../include/Network/Clients/../../Protocol/MQTT/MQTT.hpp:7:
~/repos/external/eMQTT5/lib/src/Network/Clients/../../../include/Network/Clients/../../Protocol/MQTT/../../Platform/Platform.hpp:72:2: error:
      here
#error here
 ^
1 warning and 1 error generated.
make[2]: *** [common_libs/LoadSensor/common_libs/eMQTT5/lib/CMakeFiles/eMQTT5.dir/src/Network/Clients/MQTTClient.cpp.o] Error 1
make[1]: *** [common_libs/LoadSensor/common_libs/eMQTT5/lib/CMakeFiles/eMQTT5.dir/all] Error 2
make: *** [all] Error 2

What does this mean?

Oh, link error was gone with master HEAD!! 1 hour ago, I tried it. it seems that it was my mistake. I'm sorry.
https://github.com/X-Ryl669/eMQTT5/issues/4#issuecomment-779793363
Now I can build it without any problems. Thank you.

Ok, great. The issue you experienced was because I was too fast pushing code that did not build. I've fixed up 5mn later but you've probably pulled in between. Glad it works.


Provide a type class-less encoding and decoding based API

Noah Daniels writes:

I have the following, which assumes that each record will be of length 25:

instance FromRecord Barcode where
parseRecord v
| V.length v == 25 = Barcode <$>
v .! 0  <*>
T.traverse parseField (V.tail v)
| otherwise        = mzero

> I'd like to parameterize this length, so that it can be run on datasets that might have barcodes of different lengths. But, I want to make sure that for a given data set, all barcodes are the same length. One way to do this is to infer that length from the first record in the input file; the other way is to require it as a parameter. I'm ok with either option. In some ways, I prefer requiring it as a parameter, as it keeps the biologists honest :)

> I realize this gets into dependent types, which Haskell doesn't really support. Other than the obvious and awful solution of post-processing all the records and yelling at the user if he or she gives bad input, is there any nice way to make parseRecord take the record length as a parameter? Either compile-time or run-time would be acceptable.

I think we could support this by adding a different decode function

```haskell
decodeUsing :: (Record -> Parser a) -> HasHeader -> ByteString -> Either String (Vector a)

You could then pass in an arbitrary parsing function, including a parameterized one. Here's one possible such function, assuming that you already have a FromRecord instance for a and just want to do some sanity checking:
decodeBarcode :: Int -> Record -> Parser a
decodeBarcode n v
    | V.length v - 1 /= n = mzero
    | otherwise           = parseRecord v

You'd use it like so
test = decodeUsing (decodeBarcode someLength) ...

Shouldn't this be as easy as exposing decodeWithP? I really need this as well. Let me know if that makes sense and I'll submit a PR.

Yes in principle. I wanted to do some remaining as well. I will try to do it during ICFP.


chore(up): migrate landing page

🔗 Linked issue
https://github.com/unjs/nitro/pull/1354
❓ Type of change

[ ] 📖 Documentation (updates to the documentation or readme)
[ ] 🐞 Bug fix (a non-breaking change that fixes an issue)
[X] 👌 Enhancement (improving an existing functionality like performance)
[ ] ✨ New feature (a non-breaking change that adds functionality)
[ ] ⚠ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description
This fully fixes the landing page and use latest components from Docus 2.
📝 Checklist

[X] I have linked an issue or discussion.
[X] I have updated the documentation accordingly.

Holy ... 😮


Update Github Action step codeclimate-action from 3.0.0 to 3.2.0

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
115

Relevant Lines:
115

💛  - Coveralls

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
115

Relevant Lines:
115

💛  - Coveralls


Design: OADP NFS plugin for full NFS support

Need help here:
Known issues:

[ ] Status back to the cli may be a blocker here.  Download-requests.

Right. For a while, we've said that we just need a filesystem object store plugin, but to do that we'll have to resolve the download request issue -- i.e. anything backed by an object store plugin needs to allow for velero to provide the client a URL to download the logs, tarball, etc. Basically anywhere that a client works (velero pod, user laptop, etc.), that client needs to be able to download these backup/restore artifacts via the URL retrieved from the download request.

https://issues.redhat.com/browse/RHSTOR-3545

@weshayutin Are there any details about OCS NFS support that make it relevant to this task? What's needed is to implement the object store API for an (out-of-cluster) NFS back end, and to figure out how to make download requests work with NFS (possibly requiring some sort of additional controller, on top of a plugin). I don't know that OCS supporting in-cluster NFS helps us with either one of these.


documentation errors when describing these

Following on in the vein of #171:
The "these" argument is documented as having something to do with properties in multiple places.  But, does it?  In what way does @property not work with @attr.s already?  This program works exactly as I expect it would:
import attr

@attr.s
class propertized(object):
    _x = attr.ib()
    @property
    def x(self):
        return self.x ** 2

p3 = propertized(3)
p3prime = propertized(x=3)
print(p3 == p3prime)
print(p3)

It also describes the class body as being "ignored".  This is accurate in the extremely narrow sense of the implementation of what exactly @attr.s does - either reading the attr.ib objects out of the class namespace or not, which were presumably defined during the class body - but it is somewhat unclear for users who want to know what attr.s's interface is rather than the mechanics of its internals.
The passive voice of "will be ignored" also suggests that everything will ignore the class body - attrs, python, random passers-by - whereas the docs mean to rather specifically say<EMAIL_ADDRESS>will ignore the class body".  I think it should be even more specific: attr.s will not transform any attr.ib declarations into arguments or parts of the repr, leaving them instead as private _CountingAttr objects.  I'd even go so far as to say that attr.s should emit a warning if any such objects are found, but that's probably a more complex discussion.
Also, properties is not an identifier and should not be typeset as such; "property objects" or somesuch would be a better way to express that (if it needs to be expressed).

Yeah this should be fixed before 17.1.   It would be great if you could do it since you already put all the thinking into it. :)


Create plugin-swamp.yml

Description
This is for the plugin https://github.com/jenkinsci/swamp-plugin
And hosting request is https://issues.jenkins-ci.org/browse/HOSTING-331
@capotej
@michaelneale

Permissions pull request checklist

Always

[ x] Add link to plugin/component Git repository in description above

For a newly hosted plugin only

[ x] Add link to resolved HOSTING issue in description above

For a new permissions file only

[ x] Make sure the file is created in permissions/ directory
[ x] artifactId (pom.xml) is used for name (permissions YAML file).
[ x] groupId / artifactId (pom.xml) are correctly represented in path (permissions YAML file)
[x ] Check that the file is named plugin-${artifactId}.yml for plugins

When adding new uploaders (this includes newly created permissions files)

[ x] Make sure to @mention an existing maintainer to confirm the permissions request, if applicable
[x ] Use the Jenkins community (LDAP) account name in the YAML file, not the GitHub account name
[ x] All newly added users have logged in to Artifactory at least once

Not sure why I was @ mentioned on this though ...


Upgraded to Castle.Core 4.2.0 to 4.3.1.

https://github.com/castleproject/Core/releases/tag/v4.3.1

Thanks for checking, did this at 02:00 in the morning. :)

Did I miss something? The changelog still has 2 entries for the same release:
Enhancements:
- Upgraded to Castle.Core 4.2.0 to 4.3.1 (@fir3pho3nixx, #413)
...
- Upgraded Castle.Core from 4.2.0 to 4.3.0 (@fir3pho3nixx, #409)

Will fix.


feature: separate prompt for questions where no code generation is required

Feature request
Add a separate plugin prompt that does not ask LLM to generate any code.
Not sure how this would work. Different keybind to open a different type of sidebar? Multiple modes in the sidebar that can be toggled between?
Motivation
Often I find myself asking questions about the codebase without wanting any changes. The plugin prompt tends to force the LLMs to generate code even when it makes no sense to do so.
Other
No response

e.g.

there is a ask variable in the templates, https://github.com/yetone/avante.nvim/blob/8c021f36913048b9f1d00a8839ec44e9b3918033/lua/avante/templates/planning.avanterules#L10.
So we can pass in this option in AvanteAsk similar to position=right or position=left

https://github.com/yetone/avante.nvim/blob/8c021f36913048b9f1d00a8839ec44e9b3918033/lua/avante/llm.lua#L63

So its set up for it, but not used at the moment?
I quite like the toggling idea, so I'll have a go at that.
Wheres a good place to display the current mode / hint to change the mode? In a floating window like the submit hint?

See https://github.com/yetone/avante.nvim/blob/8c021f36913048b9f1d00a8839ec44e9b3918033/lua/avante/init.lua#L40 for example.

#541 now support with
AvanteAsk ask=false. I will probably add a quick command AvanteChat which is an alias for this.

#542 add AvanteChat.


Variable scoping isn't respected (use-before-declaration allowed by language server)

Environment data

Language Server version: Microsoft Python Language Server version <IP_ADDRESS>
OS and version: macOS Mojave (10.14.6)
Python version (& distribution if applicable, e.g. Anaconda): 2.7.10
vscode-python extension version: 2019.12.53195-dev

Declare var = "Hello World"
Go to line before declaration and try var.
Expected behaviour
var should not be recognized in autocomplete results by language server before var has been declared
Actual behaviour
var. provides completion results for var as a string. As you type va you get completion option for var, which should not be in scope.
Logs
Downloading https://pvsc.azureedge.net/python-language-server-beta/Python-Language-Server-osx-x<IP_ADDRESS>.nupkg...
Language server download complete
Unpacking archive... done
[Info  - 6:08:02 PM] Analysis cache path: /Users/saqadri/Library/Caches/Microsoft/Python Language Server
[Info  - 6:08:02 PM] Microsoft Python Language Server version <IP_ADDRESS>
[Info  - 6:08:02 PM] Workspace root: /Users/saqadri/jupyter
[Info  - 6:08:03 PM] GetCurrentSearchPaths /usr/bin/python
[Info  - 6:08:03 PM] Interpreter search paths:
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-darwin
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/plat-mac/lib-scriptpackages
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-tk
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/lib-dynload
[Info  - 6:08:03 PM]     /Library/Python/2.7/site-packages
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python
[Info  - 6:08:03 PM]     /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/PyObjC
[Info  - 6:08:03 PM]     /opt/yum/lib/python2.7/site-packages
[Info  - 6:08:03 PM] User search paths:
[Info  - 6:08:04 PM] Initializing for /usr/bin/python
[Info  - 6:08:13 PM] Analysis caching mode: None.
[Info  - 6:09:02 PM] Analysis caching mode: None.
[Info  - 6:09:07 PM] Analysis caching mode: None.
[Info  - 4:38:18 PM] Analysis caching mode: None.
Code Action in file:///Users/saqadri/test.py at (0, 0) - (0, 0)
Analysis of test (User) queued. Dependencies:
Analysis version 914 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.52 ms.
Analysis complete: 1 modules in 4.57 ms.
Analysis version 914 of 1 entries has been completed in 0.74 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 915 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.23 ms.
Analysis complete: 1 modules in 0.92 ms.
Analysis version 915 of 1 entries has been completed in 0.39 ms.
Completions in file:///Users/saqadri/test.py at (3, 1)
Analysis of test (User) queued. Dependencies:
Analysis version 916 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 1.17 ms.
Analysis version 916 of 1 entries has been completed in 0.76 ms.
Code Action in file:///Users/saqadri/test.py at (3, 2) - (3, 2)
Analysis of test (User) queued. Dependencies:
Analysis version 917 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.11 ms.
Analysis complete: 1 modules in 0.61 ms.
Analysis version 917 of 1 entries has been completed in 0.2 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 918 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.13 ms.
Analysis complete: 1 modules in 0.85 ms.
Analysis version 918 of 1 entries has been completed in 0.25 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 919 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.39 ms.
Analysis complete: 1 modules in 0.9 ms.
Analysis version 919 of 1 entries has been completed in 0.48 ms.
Analysis of test (User) queued. Dependencies:
Completions in file:///Users/saqadri/test.py at (3, 4)
Analysis version 920 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.43 ms.
Analysis complete: 1 modules in 1.02 ms.
Analysis version 920 of 1 entries has been completed in 0.54 ms.
Code Action in file:///Users/saqadri/test.py at (3, 4) - (3, 4)
Code Action in file:///Users/saqadri/test.py at (0, 0) - (0, 0)
Code Action in file:///Users/saqadri/test.py at (0, 0) - (3, 0)
Code Action in file:///Users/saqadri/test.py at (0, 0) - (0, 0)
Code Action in file:///Users/saqadri/test.py at (1, 0) - (1, 0)
Code Action in file:///Users/saqadri/test.py at (1, 0) - (2, 0)
Code Action in file:///Users/saqadri/test.py at (1, 0) - (5, 0)
Code Action in file:///Users/saqadri/test.py at (1, 0) - (8, 0)
Code Action in file:///Users/saqadri/test.py at (1, 0) - (13, 0)
Code Action in file:///Users/saqadri/test.py at (3, 4) - (3, 4)
Analysis of test (User) queued. Dependencies:
Analysis version 921 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.23 ms.
Analysis complete: 1 modules in 0.84 ms.
Analysis version 921 of 1 entries has been completed in 0.33 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 922 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.59 ms.
Analysis version 922 of 1 entries has been completed in 0.2 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 923 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.61 ms.
Analysis version 923 of 1 entries has been completed in 0.21 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 924 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.1 ms.
Analysis complete: 1 modules in 0.63 ms.
Analysis version 924 of 1 entries has been completed in 0.19 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 925 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.64 ms.
Analysis version 925 of 1 entries has been completed in 0.2 ms.
Completions in file:///Users/saqadri/test.py at (3, 1)
Analysis of test (User) queued. Dependencies:
Analysis version 926 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.59 ms.
Analysis version 926 of 1 entries has been completed in 0.2 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 927 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.2 ms.
Analysis complete: 1 modules in 0.71 ms.
Analysis version 927 of 1 entries has been completed in 0.29 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 928 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.67 ms.
Analysis version 928 of 1 entries has been completed in 0.21 ms.
Analysis of test (User) queued. Dependencies:
Completions in file:///Users/saqadri/test.py at (3, 5)
Analysis version 929 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.15 ms.
Analysis complete: 1 modules in 0.68 ms.
Analysis version 929 of 1 entries has been completed in 0.23 ms.
Code Action in file:///Users/saqadri/test.py at (3, 5) - (3, 5)
Analysis of test (User) queued. Dependencies:
Analysis version 930 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.13 ms.
Analysis complete: 1 modules in 0.59 ms.
Analysis version 930 of 1 entries has been completed in 0.21 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 931 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.24 ms.
Analysis complete: 1 modules in 0.73 ms.
Analysis version 931 of 1 entries has been completed in 0.32 ms.
Analysis of test (User) queued. Dependencies:
Analysis version 932 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.12 ms.
Analysis complete: 1 modules in 0.59 ms.
Analysis version 932 of 1 entries has been completed in 0.2 ms.
Code Action in file:///Users/saqadri/test.py at (3, 2) - (3, 2)
Analysis of test (User) queued. Dependencies:
Analysis version 933 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.25 ms.
Analysis complete: 1 modules in 0.82 ms.
Analysis version 933 of 1 entries has been completed in 0.35 ms.
Completions in file:///Users/saqadri/test.py at (3, 3)
Code Action in file:///Users/saqadri/test.py at (3, 3) - (3, 3)
Analysis of test (User) queued. Dependencies:
Completions in file:///Users/saqadri/test.py at (3, 4)
Analysis version 934 of 1 entries has started.
Analysis of test (User) on depth 0 completed in 0.27 ms.
Analysis complete: 1 modules in 0.82 ms.
Analysis version 934 of 1 entries has been completed in 0.36 ms.
Code Action in file:///Users/saqadri/test.py at (3, 4) - (3, 4)

Code Snippet / Additional lnformation

#Bad
-->var.
var = "Hello world"

def foo():
    # Same issue in function scope
    --> s.
    s = "temp"

This is known; the LS doesn't do dataflow analysis to this extent and doesn't recognize ordering.

@jakebailey will this change in the future?  Have users who have opted-in to the MSFT Python LS in the Python VS Code extension had feedback about this?

#313, but it hasn't been as big of a problem from the feedback as you might have expected. This may change, but I can't quote a timeline.


SUCCESSFUL.


downgrade release feedback

https://github.com/symplify/phpstan-rules/blob/5769d2a231c414756495501a3b0c87b9299e8c85/.github/workflows/downgraded_release.yaml#L16-L18
it looks like the downgrade job will checkout whatever revision is the latest one at the time of running the job.
this means if someone commits right after a tag, a commit might slip into the downgraded release which was not part of the non-downgraded one
same problem when e.g. github actions are slow or down and the actions runs a bit later as the actual tag happens

https://github.com/symplify/phpstan-rules/blob/5769d2a231c414756495501a3b0c87b9299e8c85/.github/workflows/downgraded_release.yaml#L62-L64
overwriting a existing git tag should be prevented at all costs.
most systems which learned about the previous git tag will likely not update to the newer re-tagged version.
that means some consumers will "randomly" get the downgraded version and some will get the latest version, depending on how they fetch the tags.

Hi, thanks for feedback. I agree with you.
If you find a way to improve this, PR is welcomed 👍


[Darkside] datemonthpicker

Description

Component Checklist 📝

[ ] JSDoc
[ ] Examples
[ ] Documentation / Decision Records
[ ] Storybook
[ ] Style mappings (@navikt/core/css/config/_mappings.js)
[ ] Component tokens (@navikt/core/css/tokens.json)
[ ] CSS class deprecations (@navikt/aksel-stylelint/src/deprecations.ts)
[ ] Exports (@navikt/core/react/src/index.ts and @navikt/core/react/package.json)
[ ] New component? CSS import (@navikt/core/css/index.css)
[ ] Breaking change? Update migration guide. Consider codemod.
[ ] Changeset (Format: <Component>: <gitmoji?> <Text>. E.g. "Button: :sparkles: Add feature xyz.")

Right now it has a blue border on hover (even in error state) maybe it shouldn't be like this on error? 🤔 (though it's nice to have a hover indicator).


301 test

added a url rewrite section to the web.config to redirect /Documentation/ to the homepage.
Stops users potentially seeing an 404 error page and also confirms that 301 redirects are enabled on the server for future use when reorganising.

Is it supposed to work ?
It doesn't seem to redirect when I go to an unexisting url.
Ex : docs.orchardproject.net/Documentation/Installing-Orchardblalblabla

Its working.
It was just stop http://docs.orchardproject.net/Documentation/ from showing an error saying you're not allowed to view directory listing or something similar, not to catch all 404's.
The future use of it is intended for things like renaming files or when the attached zip files are upgraded because they have the version number in them.


cannot run data.py on the specified corpus

running python data.py is causing followin error, must be because of opening it in binary mode.
Tried it with python 3.5.3 on macOS Sierra
Traceback (most recent call last):
  File "data.py", line 255, in <module>
    prepare_raw_data()
  File "data.py", line 178, in prepare_raw_data
    id2line = get_lines()
  File "data.py", line 34, in get_lines
    parts = line.split(' +++$+++ ')
TypeError: a bytes-like object is required, not 'str'

when open the file,don't use the binary mode.


CAP Editor Integration

Can you please add some docs for the new data-plugin "wis2box.data.cap_message.CAPMessageData" in https://github.com/wmo-im/wis2box/blob/main/docs/source/reference/running/data-pipeline-plugins.rst


*Instance types should inherit from some basic Instance type which has "address"

We have in our contract library a GateKeeper contract which manages permissions for all contracts.
We have a typescript function to create permissions which looks like this currently:
export async function createPermission<ContractInstance>(
  gateKeeper: GateKeeperContract,
  securedContractInstance: ContractInstance,
  permissionName: string,
  permissionManagerAddress: string,
  permissionRecipientAddress: string
) {
  const deployedGateKeeper = await gateKeeper.deployed();
  const role: string = await securedContractInstance[permissionName].call();

  await deployedGateKeeper.createPermission(
    permissionRecipientAddress,
    (securedContractInstance as any).address,
    role,
    permissionManagerAddress
  );
}

It would be much more clean if this could be:
export async function createPermission<ContractInstance extends Truffle.ContractInstance>(
  gateKeeper: GateKeeperContract,
  securedContractInstance: ContractInstance,
  permissionName: string,
  permissionManagerAddress: string,
  permissionRecipientAddress: string
) {
  const deployedGateKeeper = await gateKeeper.deployed();
  const role: string = await securedContractInstance[permissionName].call();

  await deployedGateKeeper.createPermission(
    permissionRecipientAddress,
    securedContractInstance.address,
    role,
    permissionManagerAddress
  );
}

Apart from the address field, it would also allow us to be a lot more flexible in generics.

FYI, the same goes for *Contract types, they also have an address field

So, *Instances contracts lack address property. Yeah this is def bug on our side. I am gonna fix it asap.
I am not sure about *Contracts having address field... arent they just to deploy instances? so they should not have address.  That means you can bypass .deployed() if you just need the address for a follow up migration.
Not debating if this is "clean" :), it is just something we use that works with Truffle.

@roderik i get it. Okay, all your issues should be resolved now and released as +<EMAIL_ADDRESS>also you will need to update +<EMAIL_ADDRESS>if you used it.


[BitSail][Connector] Support Assert sink connector

Is your feature request related to a problem? Please describe
Currently we support print sink and we use print in some tests, but print is not sufficient enough to verify the test result.
So we need an AssertSink to verify the test result more accurately.
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.

@liuxiaocs7 Assigned to you!

@liuxiaocs7 hello, just checking how's everything going with this issue. We have a contributor group to discuss dev issue. Please feel free to join if you are interested

Hi @garyli1019, I refer to the design of the assert connector in seatunnel, including row_rules and column_rules.
In row rules, we need to verify the number of data pieces of a job (min_row and max_row), in columns rules, we need to verify the data type and value range, including (min, max for number min_len, max_len for string and not_null for all fields), and the conf just like this, what do you think of this?
Also, I have some questions, in my opinion, sink connector V1 use DelegateFlinkWriter to handler the sink logic, i noticed that we get column conf and construct convert, so column in writer is necessary, but ColumnInfo only includes string properties which support unique, nullable, not_null(see TypeProperty), so i add column_rules in json conf, but it looks redundant, do we have a better design idea?
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-cores/bitsail-core-flink-bridge/src/main/java/com/bytedance/bitsail/core/flink/bridge/writer/delegate/DelegateFlinkWriter.java#L113-L121
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-common/src/main/java/com/bytedance/bitsail/common/model/ColumnInfo.java#L39
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-common/src/main/java/com/bytedance/bitsail/common/typeinfo/TypeProperty.java#L28-L34
The sample conf now:
{
  "job": {
    "common": {
      "cid": 0,
      "domain": "test",
      "job_id": -24,
      "job_name": "bitsail_connector_assert_test",
      "instance_id": -720,
      "user_name": "root"
    },
    "reader": {
      "class": "com.bytedance.bitsail.connector.fake.source.FakeSource",
      "total_count": 17,
      "columns": [
        {
          "name": "name",
          "type": "string"
        },
        {
          "name": "age",
          "type": "int"
        }
      ]
    },
    "writer": {
      "class": "com.bytedance.bitsail.connector.assertion.sink.AssertSink",
      "content_type": "json",
      "columns": [
        {
          "name": "name",
          "type": "string"
        },
        {
          "name": "age",
          "type": "int"
        }
      ],
      "row_rules": {
        "min_row": 5,
        "max_row": 10
      },
      "column_rules": {
        "name": {
          "not_null": true,
          "min_len": 5,
          "max_len": 20
        },
        "age": {
          "not_null": true,
          "min": 2,
          "max": 18
        }
      }
    }
  }
}

Hi @garyli1019, I refer to the design of the assert connector in seatunnel, including row_rules and column_rules.
In row rules, we need to verify the number of data pieces of a job (min_row and max_row), in columns rules, we need to verify the data type and value range, including (min, max for number min_len, max_len for string and not_null for all fields), and the conf just like this, what do you think of this?
Also, I have some questions, in my opinion, sink connector V1 use DelegateFlinkWriter to handler the sink logic, i noticed that we get column conf and construct convert, so column in writer is necessary, but ColumnInfo only includes string properties which support unique, nullable, not_null(see TypeProperty), so i add column_rules in json conf, but it looks redundant, do we have a better design idea?
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-cores/bitsail-core-flink-bridge/src/main/java/com/bytedance/bitsail/core/flink/bridge/writer/delegate/DelegateFlinkWriter.java#L113-L121
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-common/src/main/java/com/bytedance/bitsail/common/model/ColumnInfo.java#L39
https://github.com/bytedance/bitsail/blob/1a5e3f6e8b4518cd2a28e5a028395db2800c82cb/bitsail-common/src/main/java/com/bytedance/bitsail/common/typeinfo/TypeProperty.java#L28-L34
The sample conf now:
{
  "job": {
    "common": {
      "cid": 0,
      "domain": "test",
      "job_id": -24,
      "job_name": "bitsail_connector_assert_test",
      "instance_id": -720,
      "user_name": "root"
    },
    "reader": {
      "class": "com.bytedance.bitsail.connector.fake.source.FakeSource",
      "total_count": 17,
      "columns": [
        {
          "name": "name",
          "type": "string"
        },
        {
          "name": "age",
          "type": "int"
        }
      ]
    },
    "writer": {
      "class": "com.bytedance.bitsail.connector.assertion.sink.AssertSink",
      "content_type": "json",
      "columns": [
        {
          "name": "name",
          "type": "string"
        },
        {
          "name": "age",
          "type": "int"
        }
      ],
      "row_rules": {
        "min_row": 5,
        "max_row": 10
      },
      "column_rules": {
        "name": {
          "not_null": true,
          "min_len": 5,
          "max_len": 20
        },
        "age": {
          "not_null": true,
          "min": 2,
          "max": 18
        }
      }
    }
  }
}

@liuxiaocs7 Hi, thanks for elaborate your design! Your configuration design looks good to me. I couldn't come up with a better idea than this. Please go ahead with this design