Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SD get pointer block #528

Open
VonSzarvas opened this issue Jan 14, 2021 · 17 comments
Open

SD get pointer block #528

VonSzarvas opened this issue Jan 14, 2021 · 17 comments
Assignees
Labels
bug Something isn't working deployment/staged Issue is pending in staged code. Ready for verification when deployed. task/verify Bug or feature is ready to be tested and verified
Milestone

Comments

@VonSzarvas
Copy link

Add the SD pointer block and set to "get".
Notice the block is not formed to allow the result variable to be attached.
Click code and get white screen. Click compile and error or hang.

2021-01-14 15_30_18-BlocklyProp Solo

@zfi zfi self-assigned this Jan 15, 2021
@AndyLindsay
Copy link

item = SD file (get) pointer emits item = fp; It should instead emit item = ftell(fp); In propeller C, this value is negative. It might be better to use a larger routine that returns a pre-digested value that the user can apply in SD file (set) pointer.

Also, SD file (set) pointer = item emits fp = item. It should instead emit fseek(fp, item, SEEK_CUR);

@zfi zfi added the bug Something isn't working label Jan 15, 2021
@zfi
Copy link
Contributor

zfi commented Jan 15, 2021

The block geometry of the two different states of this block are incompatible. When in default the 'set' mode, the block acts like a program element. It has top and bottom connectors and accepts an input value used to set the pointer value.
Screen Shot 2021-01-15 at 3 14 35 PM

When the block is in the 'get' mode, the geometry changes completely from a program element to an input element. The top and bottom connectors are removed as is the connector for an input value.
Screen Shot 2021-01-15 at 3 14 47 PM

These two modes of operation create a number of issues when the block is attached to another block, either as an input value or as a program element, and the type of the block is changed to the alternate value. For example, when the type is 'set' and the block is attached inline with other program elements, changing the block type to 'get' causes a couple of problems.

  • The block attempts to remove the top and bottom connectors and fails to do so because the connectors are attached to other blocks.
  • The block attempts to remove the input connector and fails if there is a value or variable block attached here.

Moving from a 'get' type to a 'set' type creates similar issues in that the the block no longer acts as an input so the connection to the input of the attached block fails. Similarly, the block changes to have programatic elements with a top and bottom connection but not attached to anything.

The two states literally are completely different things. It seems to me that the 'get' element needs to be a new, separate block.

@zfi zfi added the needs discussion Issue that requires further refinement before any code changes label Jan 15, 2021
@zfi
Copy link
Contributor

zfi commented Jan 16, 2021

@AndyLindsay, I am not quite getting the exact purpose of the get pointer block mode. If the block is supposed to seek, wouldn't we need an input value to say how far to seek from the current position? As it is now, it just returns the FILE *.

@AndyLindsay
Copy link

AndyLindsay commented Jan 16, 2021

@zfi, I'm suggesting that the SD file (set) pointer = item blocks emit fseek(fp, item, SEEK_CUR); That will move the pointer to the desired position in the file. In the example, I'm using the item variable, and assuming it stores an int, but it could also be a value block.

For SD file (get) pointer, I'm suggesting that it call ftell(fp), which returns the pointer's offset in the file. So, in item = SD file (get) pointer, the item variable would receive the result of the ftell(fp) call.

@AndyLindsay
Copy link

AndyLindsay commented Jan 16, 2021

Regarding the SD file (set/get) pointer, I do agree that it would make sense to have two separate blocks: SD file set pointer = and = SD file get pointer. This would be consistent with the most basic operations in Pin states, where we have make PIN (0) = [high] and = check PIN [0]. Their functions are inverses of each other, and so they separate blocks, one with the input shape, and the other with the output shape. The SD blocks have not received much use yet, so I would not expect changing them from one to two blocks would have minimal impact.

@AndyLindsay
Copy link

@zfi, P.S. Regarding what I said about the SD file (set) pointer = item blocks emitting fseek(fp, item, SEEK_CUR), are you suggesting to use fseek(fp, item, SEEK_SET) instead? If so, I agree. Sorry, got my constants mixed up.

@zfi
Copy link
Contributor

zfi commented Jan 17, 2021

The single block will continue to offer file pointer 'get' and 'set' functionality until we refactor this into two separate blocks. The 'set' mode now uses fseek() with an anchor reference to the start of the file (SEEK_SET). The block, when configured initially to the 'set' mode will generate console error messages if the mode is changed to 'get' while the block is attached to any other block. It is this behavior that is driving the requirement for separate blocks for 'set' and 'get'.

@zfi zfi added deployment/staged Issue is pending in staged code. Ready for verification when deployed. and removed needs discussion Issue that requires further refinement before any code changes labels Jan 17, 2021
@zfi zfi added this to the Release 1.5.7 milestone Jan 17, 2021
@zfi zfi added the task/verify Bug or feature is ready to be tested and verified label Jan 17, 2021
@zfi
Copy link
Contributor

zfi commented Jan 17, 2021

This issue is addressed in v.1.5.7 released on 1/16/2021.

@zfi zfi mentioned this issue Jan 17, 2021
@zfi
Copy link
Contributor

zfi commented Jan 18, 2021

@VonSzarvas, can you confirm that the SD file get/set pointer is working more to your liking? We can close this issue if it is resolved.

@VonSzarvas
Copy link
Author

I'm not sure what GET Pointer is returning, or how SET pointer is supposed to operate. To me something seems off, but if Andy is happy then I'm good to accept my understanding is out of sync, and sure, close this issue.

In the attached example, an existing file is opened. GET pointer is 0.
Then 140 bytes are written, and now GET pointer is 7. (curiously 140 is divisible by 7!)
Finally, SET pointer set to 4, then GET pointer read again, with a result of 0.

ie. As this test shows, SET pointer doesn't set the character position, and GET pointer doesn't return the character position.

Whereas, the Blockly docs currently show : The SD file set block sets or retrieves the current file pointer (location in characters where reads or writes begin from).

Simple_SD_Test (5).svg.txt
2021-01-19 17_18_04-BlocklyProp Solo

@AndyLindsay
Copy link

@VonSzarvas and @zfi, I didn't notice the fine print where it said that ftell doesn't return meaningful values for text files. Sorry about that! The SD file get pointer will need different C code. Based on my C tests, the SD file set pointer block should work fine, but I will send an update after testing with the blocks.

@AndyLindsay
Copy link

SD file Set pointer is working nicely, here's an example using custom blocks for some of the features on their way in the next round.

image

@AndyLindsay
Copy link

cplusplus.com says this about ftell (source):

For text streams, the numerical value may not be meaningful but can still be used to restore the position to the same position later using fseek (if there are characters put back using ungetc still pending of being read, the behavior is undefined).

With that in mind, I started looking for ftell values in C. The test code below was returning zero every time. As soon as I put fgetc calls before each ftell, it seemed to return an accurate offset from the end of the file. ...up until that offset exceeds 15. Then, the number is 15 regardless. Below is example code and the output. Could this be a PropGCC issue, or just a coding error on my part?

Example code

#include "simpletools.h"

FILE *fp;
char  alpha[64];
char  digits[64];
char  string2[64];
int item;

int main() {
  sd_mount(22, 23, 24, 25);
  strcpy(alpha, "abcdefghijklmnopqrstuvwxyz");
  strcpy(digits, "0123456789");	
  strcpy(string2, "");
  
  fp = fopen("filename.txt","w");
  fclose(fp); 
  
  fp = fopen("filename.txt","r+");
  
  fwrite(alpha, 1, 26, fp);
  fseek(fp, 4, SEEK_SET);
  fwrite(digits, 1, 9, fp);

  fseek(fp, 0, SEEK_CUR);
  int c = fgetc(fp);
  print("c = %c\r", c);
  long int ftellVal = ftell(fp);
  print("ftellVal = %d\r", ftellVal);

  fseek(fp, 0, SEEK_END);
  fseek(fp, -1, SEEK_CUR);
  c = fgetc(fp);
  ftellVal = ftell(fp);
  print("c = %c\r", c);
  print("ftellVal = %d\r", ftellVal);

  fseek(fp, 0, SEEK_SET);
  c = fgetc(fp);
  ftellVal = ftell(fp);
  print("c = %c\r", c);
  print("ftellVal = %d\r", ftellVal);

  fseek(fp, 20, SEEK_SET);
  c = fgetc(fp);
  ftellVal = ftell(fp);
  print("c = %c\r", c);
  print("ftellVal = %d\r", ftellVal);

  fseek(fp, 2, SEEK_SET);
  pause(10);
  c = fgetc(fp);
  pause(10);
  ftellVal = ftell(fp);
  print("c = %c\r", c);
  print("ftellVal = %d\r", ftellVal);

   
  fseek(fp, 0, SEEK_SET);
  fread(&string2, 1, 26, fp);
  print("%s\r", string2);
  fclose(fp);

}

Output

c = n
ftellVal = -12
c = z
ftellVal = 0
c = a
ftellVal = -15
c = u
ftellVal = -5
c = c
ftellVal = -15
abcd012345678nopqrstuvwxyz

@AndyLindsay
Copy link

AndyLindsay commented Jan 20, 2021

@zfi,

The SD file set pointer feature:

  • Working fine, so let's keep that for now
  • May have some enhancement opportunities later with SEEK_CUR and SEEK_END.

The SD file get pointer feature:

  • ftell is not working -might be a PropGCC issue.
  • No luck with fgetpos either.
  • It would probably be best to temporarily disable this feature.
  • It looks like ftell is returning some kind of offset within a 16 byte buffer.

@AndyLindsay
Copy link

@zfi we might be using a PropGCC version that is earlier than the one in which this fix appeared.

@zfi
Copy link
Contributor

zfi commented Jan 20, 2021

The cloud compiler is using this version of propgcc:

ubuntu@cd4d34afc11a:/opt/parallax/bin$ ./propeller-elf-gcc --version
propeller-elf-gcc (propellergcc_v1_0_0_2411) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

What version should the cloud compiler use if it 's not this version? The next issue would be figuring out which branch of that propgcc repo to use. @PropGit, Can you lend a hand in locating a more current version of the propgcc compiler?

@AndyLindsay
Copy link

v1_0_0_2411 is the correct version.

In testing that fix, it does appear to correctly reflect the pointer after fseek, but it does not provide correct updates after fread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working deployment/staged Issue is pending in staged code. Ready for verification when deployed. task/verify Bug or feature is ready to be tested and verified
Projects
None yet
Development

No branches or pull requests

3 participants